-
Notifications
You must be signed in to change notification settings - Fork 12.8k
fix: also respect compilerOptions.moduleResolution for ending preferences #53244
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
fix: also respect compilerOptions.moduleResolution for ending preferences #53244
Conversation
Lint failures will be fixed by #53243. |
@JoshuaKGoldberg that PR landed shortly after you opened this one, you can sync with main to resolve the issue :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
--moduleResolution nodenext
doesn’t mean that extensions are required; it just means that extensions are required in ESM files. What you have in your test are CJS files, where extensions are not required. The refactor should infer from the existing code that extensions are preferred, but it’s incorrect to infer that preference purely based on the module resolution setting. I’m not sure what the issue is in #53244, but most likely the refactor isn’t passing the resolutionMode
correctly.
TIL, thanks for the explanation!
Interesting - looks from 3505fb9 that TypeScript already is.
Hmm, looking through the
Edit: actually, I'm confused - why is this not the case? I thought with |
Looks like the bug was a duplicate of #50710 and already fixed by #51702.
I’m confused about what you’re confused about 😆
In these scenarios, you’re allowed to have an aesthetic preference as to whether you include file extensions on your relative module specifiers, since they work either way. This is what the VS Code preference controls (and what we’ll try to infer from existing usage). In Node ESM imports, you’re not allowed to omit extensions, so the preference doesn’t come into play. You just have to have extensions, no matter what the VS Code preference says, and if auto-imports or whatever fails to add it, that’s a bug. Does that clear things up? |
Got it, thanks - that's very helpful! |
The root issue was that TypeScript only looks for user preferences or a
package.json
"type": "module"
when determining whether to add.js
to extensions. Some projects don't have either of those set, but still use"compilerOptions": "node16"
or similar in their TSConfig.Fixes #52886