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

JSDoc @import not detecting use in @callback #58368

Open
turadg opened this issue Apr 29, 2024 · 7 comments
Open

JSDoc @import not detecting use in @callback #58368

turadg opened this issue Apr 29, 2024 · 7 comments
Assignees
Labels
Needs Investigation This issue needs a team member to investigate its status.

Comments

@turadg
Copy link

turadg commented Apr 29, 2024

🔎 Search Terms

jsdoc import @import callback lsp

🕗 Version & Regression Information

5.5 beta

  • I was unable to test this on prior versions because it's a new feature in 5.5. It was appearing in nightlies and I waited for the beta to confirm it's still there.

⏯ Playground Link

No response

💻 Code

/**
 * @import {Checker, Passable} from '@endo/pass-style';
 */

/**
 * @callback GetChecker
 * @param {Passable} payload
 * @returns {Checker}
 */

/**
 * @typedef {object} Misc
 * @property {Passable} passable
 */

🙁 Actual behavior

The Checker import is dim in an IDE and hovering says,

'Checker' is declared but never used.ts(6196)

🙂 Expected behavior

LSP detects that Checker is in the return of GetChecker.

Additional information about the issue

The use is detected in @typedef and @type. E.g. as with Passable in the example code.

@a-tarasyuk
Copy link
Contributor

I attempted to reproduce this issue, but it appears to be working correctly. Could you please provide additional details?

Screenshot 2024-05-01 at 01 18 40

@turadg
Copy link
Author

turadg commented Apr 30, 2024

Thanks for trying. Looking into it further, in my case it's reporting ts(6133) until I navigate to the definition. It resets again to thinking it's unused when I restart TS.

Here is a video demonstrating in VS Code.

Version: 1.88.1
Commit: e170252f762678dec6ca2cc69aba1570769a5d39
Date: 2024-04-10T17:43:08.196Z
Electron: 28.2.8
ElectronBuildId: 27744544
Chromium: 120.0.6099.291
Node.js: 18.18.2
V8: 12.0.267.19-electron.0
OS: Darwin arm64 23.4.0

@turadg
Copy link
Author

turadg commented May 7, 2024

More info: I think this only happens when the type is available by ambients. Then the imported type reports that it's not used.

This may not be a bug per current spec but it is a bit of a UX hazard because ambient resolution can differ by build order etc. So a type that's "not used" at coding time may be necessary at build time. The message that says it's not used suggests it can be removed, but that breaks the build.

@RyanCavanaugh RyanCavanaugh added the Needs More Info The issue still hasn't been fully clarified label Jun 18, 2024
@RyanCavanaugh
Copy link
Member

Looks like a bug, but I can't repro this either. We can investigate if there's a way to reproduce it

@michaeljaltamirano
Copy link

michaeljaltamirano commented Jun 20, 2024

@RyanCavanaugh I am having trouble putting together a reproduction, but the details of my case somewhat match up with this issue. However, I noticed that if I move the JSDoc import statement to happen before the last non-JSDoc import, it removes the ts(6133) error:

Before:
Screenshot 2024-06-20 at 4 36 46 PM

After:
Screenshot 2024-06-20 at 4 37 15 PM

I only saw this "unused import" error in some files. Not sure what the root cause might be, there was no pattern (relative imports vs. absolute imports, etc.) I could see in the cases I went through.

@remcohaszing
Copy link

Here’s a reproduction: playground

It’s based on this commit, where it happens in packages/language-service/lib/mdast-utils.js and packages/language-service/lib/tsconfig.js in seemingly arbritrary situations.

Rich-Harris added a commit to Rich-Harris/dts-buddy that referenced this issue Jun 21, 2024
Rich-Harris added a commit to Rich-Harris/dts-buddy that referenced this issue Jun 21, 2024
* support 5.5

* add jsdoc-import test

* use import comments

* update

* remove

* install 5.5.2

* try this i guess?

* or this?

* fix

* fix test

* skipLibCheck

* export types

* make types.d.ts a module

* export namespace

* apply workaround from microsoft/TypeScript#58368
@RyanCavanaugh RyanCavanaugh added Needs Investigation This issue needs a team member to investigate its status. and removed Needs More Info The issue still hasn't been fully clarified labels Jun 24, 2024
@RyanCavanaugh RyanCavanaugh modified the milestone: TypeScript 5.6.0 Jun 24, 2024
jaydenseric added a commit to jaydenseric/graphql-react that referenced this issue Jul 12, 2024
See:

https://devblogs.microsoft.com/typescript/announcing-typescript-5-5/#the-jsdoc-import-tag

Note that the JSDoc type imports have to be before the real module imports to workaround this TypeScript bug where sometimes the type imports are falsely considered unused:

microsoft/TypeScript#58368 (comment)
microsoft/TypeScript#58969
jaydenseric added a commit to jaydenseric/graphql-upload that referenced this issue Oct 4, 2024
See:

https://devblogs.microsoft.com/typescript/announcing-typescript-5-5/#the-jsdoc-import-tag

Note that the JSDoc type imports have to be before the real module imports to workaround this TypeScript bug where sometimes the type imports are falsely considered unused:

microsoft/TypeScript#58368 (comment)
microsoft/TypeScript#58969
jaydenseric added a commit to jaydenseric/find-unused-exports that referenced this issue Oct 10, 2024
While issues have not yet been discovered in this project, JSDoc type imports have to be before the real module imports to workaround this TypeScript bug where sometimes the type imports are falsely considered unused:

microsoft/TypeScript#58368 (comment)
microsoft/TypeScript#58969
@lishaduck
Copy link

lishaduck commented Oct 31, 2024

I had a file with no value imports, so I messed around with it and found that adding a semicolon or void 0 fixed it. Probably any statement. Doesn't really help, but for files without imports, try the void 0 trick (any statement works, I just like void).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Investigation This issue needs a team member to investigate its status.
Projects
None yet
Development

No branches or pull requests

7 participants