-
Notifications
You must be signed in to change notification settings - Fork 191
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
Exports field trees with top-level conditions result in a misleading error message #325
Comments
WorkaroundIf you are affected by this bug, you can instruct Webpack to force enhanced-resolve to use a specific condition, by duplicating the information in In my case, to force enhanced-resolve to use the "exports": {
"import": {
".": "./esm/index.js",
"./*": "./esm/*.js"
},
"require": "./build/bundle.js"
},
"basicExports": {
".": "./esm/index.js",
"./*": "./esm/*.js"
}, Then, in my config.resolve.exportsFields = ['basicExports', 'exports'] Which results in |
Can you provide small reproducible example to investigate? |
@alexander-akait thanks for your quick reply! You'll find a MWE here: https://github.com/Sidnioulz/BUGREPORT-enhanced-resolve-exports. You'll need to run |
That is interesting, because if you run code without webpack, i.e. just |
But if you change it to:
You will get:
Expected, because we don't have |
Please use:
If you use Or avoid Note - |
Just some additional context: we do manage extension matching in our Webpack and Rollup configs. Our node callees always use the CJS code for now which allows us to not go full module, and to import CJS code in our ESM builds (also covered by our Rollup / Webpack configs). Also, all our JSX, TS and TSX code is transpiled to JS before shipping. So, hopefully the adding or removing of file extensions should be a separate issue (though it does pollute my MWE a little bit, sorry for that). (Also, we don't support CJS subpaths as we export our code in ESM format, so it wouldn't work without transpiling, and only our backend projects use require() so they don't want to add babel for that; the reason we're doing dual builds is for better code splitting in our frontend than the terser can achieve on CJS code, so we don't care about this limitation. This is why my MWE did not account for this scenario, though you are right that it will not work). So, the Webpack doc link I provided in the report (https://webpack.js.org/guides/package-exports/#providing-commonjs-and-esm-version-stateful) nests conditional exports, and the Node.js doc (https://nodejs.org/api/packages.html#nested-conditions) also has nested exports, but in both cases, there is no subpath handling at the end. What you are saying is that, if I want to use both conditional exports and subpath exports, the subpaths have to be top-level. Did I understand properly? I can't find this info on the Node.js doc but, if that is how it is supposed to work, should I open reports against Webpack and Node.js documentation to clarify this? Or is it still worth doing the mapping from my MWE code to yours in enhanced-resolve (I might not be the only one who interprets the doc that way)? |
At the very least, if you decide that the bug report should be closed, would it be possible to detect this edge case and provide a more precise error message? It took me a whole day of debugging and reading source code to find why this was failing, so earlier detection would be very useful. |
As you can see it is impossible due Node.js logic, we don't add something new here, just re-implement logic
I think will be great to improve it on both sides - Node.js and webpack. In fact, you can always write your own plugin and make it work, but as you can imagine, this is not a good idea, since other bundlers most likely will not support it.
It is good idea. Node.js outputs misleading error message too. I usually start debugging with Node.js logic and when compare it with webpack logic, this allows you to quickly understand where the error is. Also if you don't need provide ESM support for some sub paths you can use:
|
Thank you! I'll rework the title and body of the issue to make it about improving the error message. |
When the exports field of a package has top-level conditions leading to subpath conditions, enhanced-resolve does not parse the subpath conditions. Following a discussion with the maintainer, this is expected behaviour as that syntax is not supported by Node.js either.
However, enhanced-resolve parses the exports field without failing, and later on in a Webpack compilation, Webpack fails to resolve subpath imports. Instead, enhanced-resolve should detect this syntax and throw an error message somewhere around
buildExportsFieldPathTree
. The reasoning is that it's very hard to understand why subpath imports fail otherwise, because the Webpack and Node.js documentations don't explicitly state that this use of exports is unsupported.Original ticket
(original title: Exports field trees with top-level conditions are not supported)
From my understanding, enhanced-resolve is supposed to know how to handle the
exports
field ofpackage.json
, including exports with conditional names, as there are checks for conditional mappings in the code.Everything in this ticket relates to the
lib/util/entrypoints.js
file.In the
buildExportsFieldPathTree
, we can see that exports that do not start with a dot are rejected. Conditional mappings are checked after the building of thetreeRoot
, so this means that exports with top-level conditions are wrongly assumed to be incorrect syntax. This results intreeRoot
containing a single file entry that matches only.
and has the entireexports
field as its content.package.json:
Webpack.config.js:
Obtained
treeRoot
:As a consequence, imports of the form
@org/mylib
work, but@org/mylib/foo
fail. Should you need a reproduction example, the one written in the Webpack 5 documentation does not work: https://webpack.js.org/guides/package-exports/#providing-commonjs-and-esm-version-stateful.I am not sure what the appropriate approach is, hence the lack of patch.
One could perform a conditional mapping match before building
treeRoot
, and building the first matching sub-tree, resulting in atreeRoot
like so:Or one could map conditional trees into trees with a top-level relative path and conditional leaves, resulting in something like:
Or the treeRoot structure itself could be recursive, though I've no idea what impact this would have on the rest of the code.
The text was updated successfully, but these errors were encountered: