-
-
Notifications
You must be signed in to change notification settings - Fork 6.3k
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
feat(resolve): support subpath imports #10929
Conversation
Closes vitejs#4439 More context: vitejs#10504
Bug introduced in vitejs#10683 Some packages use "require" instead of "default" for CJS entry
…in the `getInlineConditions` function.
…and avoid breaking certain `tryFsResolve` calls too (like with `es5-ext`)
The "{id}/package.json" lookup done by `resolvePackageData` is insufficient for certain edge cases, like when "node_modules/{dep}" is linked to a directory without a package.json in it. With this PR, you can now import any file from node_modules even if it has no package.json file associated with it. This mirrors the same capability in Node's resolution algorithm. In addition to supporting more edge cases, this new implementation might also be faster in some cases, since we are doing less lookups than compared to the previous behavior of calling `resolvePackageData` for every path in the `possiblePkgIds` array.
@benmccann You might want to look at only b3ef8f3 if you want to review this PR, as there are other PRs it depends on, thereby adding a bunch of noise to the "Files changed" tab. |
} | ||
|
||
// Find the importer's nearest package.json with a "name" field. | ||
// Some projects (like Svelte) have nameless package.json files to |
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 will probably change in the future with Svelte 4 when we drop support for old versions of Node, so worth including the version here
// Some projects (like Svelte) have nameless package.json files to | |
// Some projects (like Svelte 3) have nameless package.json files to |
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.
is there any reason to prefer this PR over #7770 which implements the same feature?
@@ -110,7 +110,7 @@ | |||
"postcss-import": "^15.0.0", | |||
"postcss-load-config": "^4.0.1", | |||
"postcss-modules": "^5.0.0", | |||
"resolve.exports": "^1.1.0", | |||
"resolve.exports": "npm:@alloc/resolve.exports@^1.1.0", |
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.
we don't need to change this to use a fork. the current resolve.exports
version used by Vite supports subpath imports
Description
This uses
resolveExports
to resolve subpath imports. To do so, it has to replace the leading#
with./
to mimic anexports
mapping field. With each path returned byresolveExports
, we callthis.resolve
to support mapping to external packages (the Node.js docs say this is valid).Additional context
Closes #7385
TODO: Write tests or copy them over from #7770
What is the purpose of this pull request?