refactor: drop JSDoc @param defaults#8350
Conversation
🎉 All green!❄️ No new flaky tests detected 🎯 Code Coverage (details) 🔗 Commit SHA: 16d78a1 | Docs | Datadog PR Page | Give us feedback! |
Overall package sizeSelf size: 5.82 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | import-in-the-middle | 3.0.1 | 82.56 kB | 817.39 kB | | dc-polyfill | 0.1.11 | 25.74 kB | 25.74 kB |🤖 This report was automatically generated by heaviest-objects-in-the-universe |
BenchmarksBenchmark execution time: 2026-05-07 23:45:21 Comparing candidate commit 58580c6 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 1742 metrics, 102 unstable metrics. |
watson
left a comment
watson
left a comment
There was a problem hiding this comment.
I'm surprised this value isn't used by IDEs when it has been part of the spec for a long time: https://jsdoc.app/tags-param#optional-parameters-and-default-values
I understand why you want to avoid it for cases where it's already documented in the method signature, but for cases where it's not, but the default value is set inside the method body, I don't understand why this kind of documentation is discouraged??
The goal is to make it part of the description itself. That way it is surfaced, if it is not a default value. This will make it clearer when adding it. |
Shouldn't this PR then not add it to the |
Activates `jsdoc/no-defaults`. The default values are visible from the function signature destructuring (`x = 0`) and were duplicated on the type tag. JSDoc-flavored TypeScript treats `=...` on optional params as unparsed text, so the default never reached tooling — the description-suffix shape (`Defaults to N`) already covered the cases where the default is non-obvious.
Some defaults are not visible in the function signature because they are derived from the body. Keep those values in prose so `jsdoc/no-defaults` does not remove the only rendered documentation for them. Refs: #8350 (comment)
BridgeAR
force-pushed
the
BridgeAR/2026-05-07-eslint-jsdoc-no-defaults
branch
from
58580c6 to
16d78a1
Compare
| * @param {string} [options.sourceFile] - The source file path. Defaults to `deployedFile` if not provided. | ||
| * @param {number} [options.stackIndex=0] - The stack index to use when inferring the file from the stack trace. | ||
| * @param {number} [options.stackIndex] - The stack index to use when inferring the file from the stack trace. | ||
| * Defaults to 0. |
There was a problem hiding this comment.
Here it is part of the method signature.
| * Defaults to 0. |
| * @param {number} [options.timeout] - Timeout in milliseconds before the promise rejects. Defaults to 30 seconds. | ||
| * @param {number} [options.expectedMessageCount] - Number of matching messages to wait for. Defaults to 1. | ||
| * @param {boolean} [options.resolveAtFirstSuccess] - Resolve as soon as `fn` first runs without throwing. | ||
| * Defaults to false. |
There was a problem hiding this comment.
Also here:
| * @param {number} [options.timeout] - Timeout in milliseconds before the promise rejects. Defaults to 30 seconds. | |
| * @param {number} [options.expectedMessageCount] - Number of matching messages to wait for. Defaults to 1. | |
| * @param {boolean} [options.resolveAtFirstSuccess] - Resolve as soon as `fn` first runs without throwing. | |
| * Defaults to false. | |
| * @param {number} [options.timeout] - Timeout in milliseconds before the promise rejects. | |
| * @param {number} [options.expectedMessageCount] - Number of matching messages to wait for. | |
| * @param {boolean} [options.resolveAtFirstSuccess] - Resolve as soon as `fn` first runs without throwing. |
| * @param {number} [options.expectedCount] Resolve after this many matching groups arrive. Defaults to 1. | ||
| * @param {number} [options.timeout] Timeout in milliseconds before the promise rejects. Defaults to 30 seconds. |
There was a problem hiding this comment.
And here:
| * @param {number} [options.expectedCount] Resolve after this many matching groups arrive. Defaults to 1. | |
| * @param {number} [options.timeout] Timeout in milliseconds before the promise rejects. Defaults to 30 seconds. | |
| * @param {number} [options.expectedCount] Resolve after this many matching groups arrive. | |
| * @param {number} [options.timeout] Timeout in milliseconds before the promise rejects. |
| * @param {object} moduleData | ||
| * @param {string} moduleData.path | ||
| * @param {boolean} [moduleData.internal = false] | ||
| * @param {boolean} [moduleData.internal] Defaults to false. |
There was a problem hiding this comment.
| * @param {boolean} [moduleData.internal] Defaults to false. | |
| * @param {boolean} [moduleData.internal] |
| * @param {boolean} [moduleData.internal] Defaults to false. | ||
| * @param {object} moduleData.context | ||
| * @param {boolean} [moduleData.excludeDefault = false] | ||
| * @param {boolean} [moduleData.excludeDefault] Defaults to false. |
There was a problem hiding this comment.
| * @param {boolean} [moduleData.excludeDefault] Defaults to false. | |
| * @param {boolean} [moduleData.excludeDefault] |
| * @param {string} args.name module name | ||
| * @param {string[]} [args.versions] array of semver range strings | ||
| * @param {string} [args.file='index.js'] path to file within package to instrument | ||
| * @param {string} [args.file] path to file within package to instrument |
There was a problem hiding this comment.
This isn't part of the method signature:
| * @param {string} [args.file] path to file within package to instrument | |
| * @param {string} [args.file] path to file within package to instrument. Defaults to 'index.js'. |
| * @param {string} [args.file] path to file within package to instrument | ||
| * @param {string} [args.filePattern] pattern to match files within package to instrument | ||
| * @param {boolean} [args.patchDefault=true] whether to patch the default export | ||
| * @param {boolean} [args.patchDefault] whether to patch the default export |
There was a problem hiding this comment.
Neither is this:
| * @param {boolean} [args.patchDefault] whether to patch the default export | |
| * @param {boolean} [args.patchDefault] whether to patch the default export. Defaults to true. |
| /** | ||
| * @param {object} config - Configuration object | ||
| * @param {string} [config.site='datadoghq.com'] - The Datadog site | ||
| * @param {string} [config.site] - The Datadog site |
There was a problem hiding this comment.
Nor this:
| * @param {string} [config.site] - The Datadog site | |
| * @param {string} [config.site] - The Datadog site. Defaults to 'datadoghq.com'. |
| * @param {boolean} [options.ritmReset] - Resets the Require In The Middle cache. Defaults to true. | ||
| * @param {boolean} [options.wipe] - Wipes tracer and non-native modules from require cache. Defaults to false. |
There was a problem hiding this comment.
But this is:
| * @param {boolean} [options.ritmReset] - Resets the Require In The Middle cache. Defaults to true. | |
| * @param {boolean} [options.wipe] - Wipes tracer and non-native modules from require cache. Defaults to false. | |
| * @param {boolean} [options.ritmReset] - Resets the Require In The Middle cache. | |
| * @param {boolean} [options.wipe] - Wipes tracer and non-native modules from require cache. |
2 participants
Summary
Activates
jsdoc/no-defaults. The default values are visible from thefunction signature destructuring (
x = 0) and were duplicated on thetype tag. JSDoc-flavored TypeScript treats
=...on optional params asunparsed text, so the default never reached tooling — the
description-suffix shape (
Defaults to N) already covered the caseswhere the default is non-obvious.