Repository navigation
fix(recommended): stop no-unused-vars reporting using declarations - #199
Conversation
There was a problem hiding this comment.
Should these two be bumped as well so they stay in sync?
There was a problem hiding this comment.
Yes. @typescript-eslint/types and @typescript-eslint/utils now move to ^8.46.0 along with the three dev copies.
Leaving them behind was what split the tree. types and utils were locked at 8.33.1 and hoisted, and every newer @typescript-eslint/* package pins its siblings to its own exact version, so npm nested a second full set under each one. All five are now locked at 8.58.2, the same version typescript-eslint itself already resolves to, so the tree holds one copy of each. These are regular dependencies, not peers, so raising them does not constrain consumers.
| "eslint": ">=9.19.0", | ||
| "obsidian": "1.8.7", | ||
| "typescript-eslint": "^8.35.1" | ||
| "typescript-eslint": "^8.46.0" |
There was a problem hiding this comment.
Are there special considerations for upgrading peerDependencies? I think consumers that have typescript-eslint pinned to a version below this will get an error with this change.
There was a problem hiding this comment.
Good catch. It was a real problem, just not the one I expected, so I've reverted the peer bump.
ignoreUsingDeclarations first shipped in typescript-eslint 8.46.0. The rule's option schema has additionalProperties: false, so an older version rejects the whole config instead of ignoring the unknown key. That's why I raised the floor.
I checked what a consumer on 8.45.0 actually gets. npm does not refuse the install. typescript-eslint is both a dependency and a peer of this plugin, so npm quietly nests its own newer copy under the plugin. The recommended config alone then lints fine. But a consumer who layers their own typescript-eslint configs on top, as docs/configuration.md shows, fails with:
ConfigError: Config "typescript-eslint/base": Key "plugins": Cannot redefine plugin "@typescript-eslint".
So I dropped the floor bump. The dependency and the peer are back at ^8.35.1. The option is now set only when the installed rule's schema declares it (lib/ruleOptions.ts). On 8.35 to 8.45, using declarations are still reported, exactly as before this PR. From 8.46.0 on, they're ignored. Here is the packed tarball checked against a consumer on each version, using the layered config from the docs:
- 8.45.0:
typescript-eslintdedupes to the consumer's copy and the config loads. Unusedusingbindings are still reported. - 8.46.0: the same config loads, and only the genuinely dead
const _dead = 1is reported.
The dev copies stay at ^8.46.0, because the tests pass the option to the rule directly.
| "version": "8.33.1", | ||
| "resolved": "https://registry.npmjs.org/@typescript-eslint/eslint-plugin/-/eslint-plugin-8.33.1.tgz", | ||
| "integrity": "sha512-TDCXj+YxLgtvxvFlAvpoRv9MAncDLBV2oT9Bd7YBGC/b/sEURoOYuIwLI99rjWOfY3QtDzO+mk0n4AmdFExW8A==", | ||
| "version": "8.70.1", |
There was a problem hiding this comment.
Where is 8.70.1 coming from? This is different than the version specified in the package.json.
There was a problem hiding this comment.
It was whatever ^8.46.0 resolved to on the day I reinstalled. 8.70.1 was the latest 8.x at the time. The lockfile records the resolved version, not the range. It still wasn't right, though. typescript-eslint itself stayed locked at 8.58.2 and carried its own nested 8.58.2 set, so the tree held two versions of every @typescript-eslint/* package.
I've re-locked them. Every @typescript-eslint/* entry is now 8.58.2, the version typescript-eslint was already locked at, so the lockfile diff only removes the old 8.33.1 set and the nested duplicates. That also brought back master's expected output for the await-thenable suggestion test. The earlier change there was only a whitespace difference in 8.70.1.
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The configuration, dependency requirements, and regression coverage are consistent with the intended behavior.
Review effort: Balanced
Findings: None
What changed in this PR
Updates the recommended TypeScript ESLint configuration to allow unused using declarations.
Changes:
- Enables
ignoreUsingDeclarations. - Adds
usingandawait usingcoverage. - Raises TypeScript ESLint dependency floors and updates test output.
| File | Description |
|---|---|
lib/index.ts |
Applies shared unused-variable options. |
lib/ruleOptions.ts |
Defines the new rule options. |
tests/typescriptEslintRules.test.ts |
Tests declaration handling. |
tests/typescriptEslintTypeCheckedRules.test.ts |
Updates suggestion output. |
package.json |
Raises dependency floors. |
package-lock.json |
Updates resolved dependencies. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| export const noUnusedVarsOptions = [{ | ||
| args: "none", | ||
| ignoreRestSiblings: true, | ||
| ...(supportsIgnoreUsingDeclarations ? { ignoreUsingDeclarations: true } : {}), |
There was a problem hiding this comment.
This is on purpose. Raising the floor to 8.46.0 is the change I reverted after the peer question above. With typescript-eslint both a dependency and a peer, a consumer pinned below 8.46.0 gets a second nested copy, and the layered config from docs/configuration.md then fails with Cannot redefine plugin "@typescript-eslint".
So a consumer on 8.35 to 8.45 keeps exactly the behaviour they have today, and anyone on 8.46.0 or later, which is what a fresh install resolves to, gets the fix. Upgrading typescript-eslint is enough to pick it up.
A `using` / `await using` binding is consumed through `Symbol.dispose` at block exit, yet the syntax still requires a name, so the recommended `@typescript-eslint/no-unused-vars` entry reported every one of them as assigned but never used. Enable `ignoreUsingDeclarations`. The option first shipped in typescript-eslint 8.46.0, and the rule's schema rejects unknown keys, so an older copy would fail the whole config as invalid. The dependency and peer floors of `typescript-eslint` move to ^8.46.0, and the dev copies used by the rule tests move with them. The options now live in `ruleOptions.ts` so the test exercises exactly what the recommended config ships. `varsIgnorePattern` is deliberately not added: an underscore-prefixed dead binding is still reported. The newer typescript-eslint trims the whitespace left behind by the await-thenable `removeAwait` suggestion, so that pinned output is updated.
Raising the typescript-eslint floor to 8.46.0 did not make npm refuse an older copy. Because the package is also a regular dependency, npm quietly nested a second copy under the plugin. A consumer who layers their own tseslint configs, as docs/configuration.md shows, then got "Cannot redefine plugin @typescript-eslint". Keep the floor where it was. Set ignoreUsingDeclarations only when the installed no-unused-vars schema declares it. On 8.35-8.45 a using declaration is still reported, as before this change. Lock every @typescript-eslint package at 8.58.2, the version typescript-eslint was already locked at, so the tree holds one copy. Move the types and utils dependencies to ^8.46.0 alongside the dev copies. At 8.58.2 the await-thenable suggestion still leaves the double space, so that test output goes back to master's.
|
@saberzero1 what do you think of this PR? I'd like to merge this once you approve instead of https://github.com/obsidianmd/community-workers/pull/52 |
A
using/await usingbinding is consumed throughSymbol.disposeat block exit, but the syntax still requires a name. The recommended@typescript-eslint/no-unused-varsentry therefore reported every one of them as assigned but never used. This PR enablesignoreUsingDeclarations.The option first shipped in typescript-eslint 8.46.0, and the rule's schema rejects unknown keys. The plugin keeps its
typescript-eslintfloor at^8.35.1and sets the option only when the installed rule's schema declares it. On older versions,usingdeclarations are still reported, as before.The options now live in
ruleOptions.ts, so the test exercises exactly what the recommended config ships.varsIgnorePatternis deliberately not added, so an underscore-prefixed dead binding is still reported.The
@typescript-eslint/*dependencies and dev dependencies move to^8.46.0, since the tests pass the option directly. All of them are locked at 8.58.2, the versiontypescript-eslintalready resolved to.