Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

imports incorrectly marked as unused when augmenting the module it was imported from in .d.ts file #54042

Closed
DetachHead opened this issue Apr 27, 2023 · 3 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@DetachHead
Copy link
Contributor

Bug Report

πŸ”Ž Search Terms

unused import augment module

πŸ•— Version & Regression Information

5.1.0-dev.20230427

⏯ Playground Link

Playground link with relevant code

πŸ’» Code

import { Program } from 'typescript'

declare module 'typescript' {
    export const foo: Program
}

πŸ™ Actual behavior

'Program' is declared but its value is never read. (6133)

πŸ™‚ Expected behavior

no warning

@RyanCavanaugh
Copy link
Member

Module exports are implicitly in scope inside a module augmentation, so the import literally is unused.

Demonstration of this fact:

import { Program as Program2 } from 'typescript'
declare module 'typescript' {
    // No error
    export const foo: Program;
    // No error
    export const bar: ScriptKind;
}

@RyanCavanaugh RyanCavanaugh added the Working as Intended The behavior described is the intended behavior; this is not a bug label Apr 27, 2023
@DetachHead
Copy link
Contributor Author

@RyanCavanaugh i thought that too, but removing the import caused an error:

declare module 'typescript' {
    export const foo: Program // Cannot find name 'Program'. (2304)
}

but playing around with it more i noticed that adding an export {} fixes it, so i guess this just another weird thing that non-module files do?

@typescript-bot
Copy link
Collaborator

This issue has been marked 'Working as Intended' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

Krinkle added a commit to qunitjs/qtap that referenced this issue Jan 9, 2025
Workarounds for TypeScript shortcomings:

* `@typedef` can't be namespaced in JS files, thus mandating awkward
  constructs like `/** @import { Logger } from './qtap.js' */`.

* There is no way to apply a type to a `catch` clause in JS files.
  The workaround is to re-assign it to a local variable and type that
  like `catch (e) { /* @type {any} */ const err = e;`.

* There is no way to disable type checking for an entire block,
  function, or otherwise multiple lines, thus mandating lots of
  `// @ts-ignore` lines for the same problem, or to move the code
  out into a separate file where the entire file is ignored (as I
  ended up doing for client.js).

  Reported 8y ago, microsoft/TypeScript#19573.

* `@types/node` lacks Error.code or Error.stack definitinos,
  thus regularly requiring use of the above workarounds.
  Reported 3y ago, DefinitelyTyped/DefinitelyTyped#59692.

* The `noUnusedLocals` rule is broken, making a paradoxal claim.

  ```
  /** @import { Logger } from './qtap.js' */

  /**
    * @param {Logger} logger
    */
  ```
  The above makes sense, and passes, until noUnusedLocals is enabled.
  Then it fails:

  ```
  error TS6133: 'Logger' is declared but its value is never read.
  /** @import { Logger } from './qtap.js' */
  ```

  Removing the import, of course, causes a more severe error instead:
  ```
  error TS2304: Cannot find name 'Logger'.
  * @param {Logger} logger
  ```

  Reported 2y ago, closed as wontfix.
  microsoft/TypeScript#54042

* The `noUnusedParameters` errors on unused arguments before arguments
  that are required and used. It appears to be unconfigurable and thus
  serves no purpose. Leave disabled in favor of ESLint no-unused-vars.
Krinkle added a commit to qunitjs/qtap that referenced this issue Jan 11, 2025
Workarounds for TypeScript shortcomings:

* `@typedef` can't be namespaced in JS files, thus mandating awkward
  constructs like `/** @import { Logger } from './qtap.js' */`.

* There is no way to apply a type to a `catch` clause in JS files.
  The workaround is to re-assign it to a local variable and type that
  like `catch (e) { /* @type {any} */ const err = e;`.

* There is no way to disable type checking for an entire block,
  function, or otherwise multiple lines, thus mandating lots of
  `// @ts-ignore` lines for the same problem, or to move the code
  out into a separate file where the entire file is ignored (as I
  ended up doing for client.js).

  Reported 8y ago, microsoft/TypeScript#19573.

* `@types/node` lacks Error.code or Error.stack definitinos,
  thus regularly requiring use of the above workarounds.
  Reported 3y ago, DefinitelyTyped/DefinitelyTyped#59692.

* The `noUnusedLocals` rule is broken, making a paradoxal claim.

  ```
  /** @import { Logger } from './qtap.js' */

  /**
    * @param {Logger} logger
    */
  ```
  The above makes sense, and passes, until noUnusedLocals is enabled.
  Then it fails:

  ```
  error TS6133: 'Logger' is declared but its value is never read.
  /** @import { Logger } from './qtap.js' */
  ```

  Removing the import, of course, causes a more severe error instead:
  ```
  error TS2304: Cannot find name 'Logger'.
  * @param {Logger} logger
  ```

  Reported 2y ago, closed as wontfix.
  microsoft/TypeScript#54042

* The `noUnusedParameters` errors on unused arguments before arguments
  that are required and used. It appears to be unconfigurable and thus
  serves no purpose. Leave disabled in favor of ESLint no-unused-vars.

* The `noImplicitAny: false` setting, while ostensibly turning some
  checks off, makes array behaviour more strict, changing default
  from `any[]` to an awkward `never[]`.

  Reported 5y ago, https://stackoverflow.com/a/57563877/319266

  ```
  const seen = { plan: null, asserts: [] };

  p.on('assert', function (result) {
    //> error TS2345: Argument of type 'any' is not assignable to parameter of type 'never'.
    seen.asserts.push(result);
  })
  ```
Krinkle added a commit to qunitjs/qtap that referenced this issue Jan 25, 2025
Workarounds for TypeScript shortcomings:

* `@typedef` can't be namespaced in JS files, thus mandating awkward
  constructs like `/** @import { Logger } from './qtap.js' */`.

* There is no way to apply a type to a `catch` clause in JS files.
  The workaround is to re-assign it to a local variable and type that
  like `catch (e) { /* @type {any} */ const err = e;`.

* There is no way to disable type checking for an entire block,
  function, or otherwise multiple lines, thus mandating lots of
  `// @ts-ignore` lines for the same problem, or to move the code
  out into a separate file where the entire file is ignored (as I
  ended up doing for client.js).

  Reported 8y ago, microsoft/TypeScript#19573.

* `@types/node` lacks Error.code or Error.stack definitinos,
  thus regularly requiring use of the above workarounds.
  Reported 3y ago, DefinitelyTyped/DefinitelyTyped#59692.

* The `noUnusedLocals` rule is broken, making a paradoxal claim.

  ```
  /** @import { Logger } from './qtap.js' */

  /**
    * @param {Logger} logger
    */
  ```
  The above makes sense, and passes, until noUnusedLocals is enabled.
  Then it fails:

  ```
  error TS6133: 'Logger' is declared but its value is never read.
  /** @import { Logger } from './qtap.js' */
  ```

  Removing the import, of course, causes a more severe error instead:
  ```
  error TS2304: Cannot find name 'Logger'.
  * @param {Logger} logger
  ```

  Reported 2y ago, closed as wontfix.
  microsoft/TypeScript#54042

* The `noUnusedParameters` errors on unused arguments before arguments
  that are required and used. It appears to be unconfigurable and thus
  serves no purpose. Leave disabled in favor of ESLint no-unused-vars.

* The `noImplicitAny: false` setting, while ostensibly turning some
  checks off, makes array behaviour more strict, changing default
  from `any[]` to an awkward `never[]`.

  Reported 5y ago, https://stackoverflow.com/a/57563877/319266

  ```
  const seen = { plan: null, asserts: [] };

  p.on('assert', function (result) {
    //> error TS2345: Argument of type 'any' is not assignable to parameter of type 'never'.
    seen.asserts.push(result);
  })
  ```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

3 participants