-
Notifications
You must be signed in to change notification settings - Fork 764
Port TypeScript PR #62844: Gate #/ subpath imports on NodeNext and Bundler
#2330
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
Conversation
…ndler Co-authored-by: RyanCavanaugh <[email protected]>
Co-authored-by: RyanCavanaugh <[email protected]>
#/ subpath imports on NodeNext and Bundler
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.
Pull request overview
This PR ports TypeScript PR #62844, which enables #/ root wildcard imports in package.json imports fields for NodeNext and Bundler module resolution modes. This aligns with Node.js PR nodejs/node#60864, allowing symmetric patterns between exports and imports fields.
Key changes:
- Adds a new
NodeResolutionFeaturesImportsPatternRootfeature flag to control#/root wildcard support - Updates
NodeNextDefaultandBundlerDefaultto include the new flag, whileNode16Defaultexcludes it for backward compatibility - Modifies the import resolution logic to conditionally allow
#/patterns based on the feature flag
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| internal/module/types.go | Defines the new NodeResolutionFeaturesImportsPatternRoot flag and includes it in the appropriate mode defaults (NodeNext, Bundler, All) while excluding it from Node16 for backward compatibility |
| internal/module/resolver.go | Updates loadModuleFromImports() to conditionally reject #/ imports only when the feature flag is not set, allowing the pattern in NodeNext and Bundler modes |
|
|
||
| func (r *resolutionState) loadModuleFromImports() *resolved { | ||
| if r.name == "#" || strings.HasPrefix(r.name, "#/") { | ||
| if r.name == "#" || (strings.HasPrefix(r.name, "#/") && (r.features&NodeResolutionFeaturesImportsPatternRoot) == 0) { |
Copilot
AI
Dec 10, 2025
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.
This change enables #/ root wildcard imports for NodeNext and Bundler modes but lacks test coverage. According to the project guidelines, at least one minimal test case should be added to verify the new behavior works correctly in each mode (Node16 rejects, NodeNext/Bundler accepts). Consider adding a test in testdata/tests/cases/compiler/ that demonstrates:
#/pattern being rejected in Node16 mode#/pattern being accepted in NodeNext mode#/pattern being accepted in Bundler mode
The test should include a package.json with an imports field like "#/*": "./src/*" to match the example from the PR description.
|
Sad there's no test but it's correct, so |
RyanCavanaugh
left a comment
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.
@copilot also copy the compiler testcases from the original PR. Do not port the fourslash test
Co-authored-by: RyanCavanaugh <[email protected]>
Added the compiler test cases from the original PR in commit 400b4d7. The tests verify that:
Did not port the fourslash test as requested. |
jakebailey
left a comment
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.
A bit sad I didn't notice that the original PR didn't just use one test with node16,nodenext,bundler, but, oh well
Thanks for the feedback! I intentionally separated the error cases into a distinct test file because I thought it would make the error baselines cleaner and easier to read. But now I'm wondering if that separation was actually necessary. For future reference, would it be better to combine them into a single test with node16,nodenext,bundler? I'd appreciate any guidance! |
|
Yeah, it makes separate baselines, so is a pretty good solution to this. |
Port microsoft/TypeScript PR #62844 - Gate
#/subpath imports on NodeNext and Bundler module resolution modesThis PR aligns with Node.js PR nodejs/node#60864, which allows defining symmetric
exportsandimportsfields in package.json:{ "exports": { "./*": "./src/*" }, "imports": { "#/*": "./src/*" } }Changes
NodeResolutionFeaturesImportsPatternRootflag (bit 6) to gate#/root wildcard importsNodeNextDefaultandBundlerDefaultto include the new flag, whileNode16Defaultexcludes itloadModuleFromImports()to check feature flag before rejecting#/patternsTest Coverage
Added two test cases:
nodeModulesPackageImportsRootWildcard.ts- Tests that#/imports work in NodeNext modenodeModulesPackageImportsRootWildcardNode16.ts- Tests that#/imports are rejected in Node16 modeBehavior
#/importsImports like
#cjsor#utils(not starting with#/) continue to work in all modes.Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.