-
-
Notifications
You must be signed in to change notification settings - Fork 186
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
Support ESM resolution #222
Comments
Some tools/bundlers use This came up in react-native-community/cli#1168 where dependencies that don't explicitly export I'm not aware of other problematic tools right now, it's just something we should keep in mind. See also: nodejs/node#33460 |
The plan is that you'll be able to explicitly request |
For the "I need to load const {sync: pkgUp} = require('pkg-up');
const packagePath = pkgUp({ cwd: require.resolve('uuid') }); https://github.com/sindresorhus/pkg-up Could trivially be packaged up into a I guess it could be wrong if people have nested |
@SimenB thanks for this suggestion! I took the liberty to copy it over to nodejs/node#33460 (comment) which might be a better place to continue this discussion. This definitely looks like an elegant solution, but of course it doesn't answer the question yet if the current behavior of node is desired at all 😉 . |
@SimenB the edge cases very much matter here; we need a way that works in all cases to identify the directory that a package resolves to, and with an "exports" that doesn't expose I also don't think that's an edge case - that's "every package that uses exports and babel", which is likely to be the 99% case. |
I don't think I've ever seen a nested package.json. Regardless, this isn't the correct issue to discuss it |
|
@SimenB does this count as a nested |
There you go, real usage. 😀 Still, it's off topic for this issue |
If a library adds an "exports" that does not include the "package.json", isn't that a breaking change and conscious decision from the library? It's the same as if the library adds a I'm not sure how extended
|
@franciscop yes, it is. Adding "exports" is almost always a breaking change, unless you preserve every possible entry point (use |
I noticed @ljharb's comment on Rollup repo mentioning that ESM resolution API is intended to be async only. Just want to point out that there are use cases for a synchronous implementation. Mine is: I'm transpiling ESM to CommonJS using Babel, replacing I understand providing both sync and async implementations may be too complicated to be viable, but just wanted to flag that some use cases do exist. |
node doesn’t have synchronous ESM resolution, because not every ESM specifier is possible to synchronously resolve. I wish this wasn’t the case, but babel needs a way for plugins to run asynchronously anyways, so hopefully this will help encourage that. |
I thought Node's ESM loader has to be async due to top level await, but I wasn't aware of a reason why resolution of specifiers can't be synchronous as it's all static analysis. Am I wrong? For my use case I'm OK not to support top level await. I'm aware that's not a perfect ESM to CJS translation, but it's close enough. |
For filesystem specifiers, it can be. However, node wants to preserve the ability to later add, for example, https imports - and thus, so too must resolve. |
@ljharb Thanks for reply. Ah, interesting. I hadn't though of that. For HTTPS import, would resolution be searching up the folder hierarchy for a i.e. would If not, resolution could be sync - it'd resolve straight away to Resolving identifiers within the file would obviously require a network round trip as the contents of the file are needed, but as I understand it, that's outside the scope of this package - it aims only to resolve paths. Please excuse me if I'm missing something again. |
To be honest, it's very unclear. I personally think https imports are a horrifically bad idea, and i hope node never supports them, but the main thing is that until node's path is clear i need to keep resolve open to matching it. |
@ljharb Thanks for explaining. Makes sense now! |
That said... it seems like a pretty niche case. So if it were practical to provide a sync version (with an explicit warning that it does not support async resolution like https import), it could still be useful in the majority of cases. I am using enhanced-resolve (Webpack's implementation) for now, which does provide sync resolution. But my understanding is that everyone will switch to I'll stop banging on about this now! |
As more and more packages take the (IMO user-hostile) approach of dropping the Is support for exports being worked on currently? Is it blocked by other packages? Have many tools moved on to alternatives like I'm not trying to apply any pressure, but if the maintainers were able to provide an update on the current situation and any kind of guidance on the next steps to be taken and whether there is a plan to take them, I'd appreciate it. |
@IanVS yes, it's something i very much want to complete. I'll try to prioritize it this month. |
What if I just write a new resolver? I have a lot of experience now with generator-based architectures. Generators are the most modern/decent way to write code once and have it work in both sync and async contexts, and sync generators have good environment support these days. I think I can create something that will be quite a bit more friendly to use than |
@ljharb Would you be interested in collaborating on something like that? |
I am highly disinclined to use generators; i find them horrific to maintain. I’d prefer to adapt the current code to support “exports” rather than try a rewrite (something that is virtually always a disaster). Additionally, using generators or iterators would mean we couldn’t support down to node 0.4, which is absolutely critical for this package. |
OK, that makes sense re 0.4. I still think there's value in creating a radically simpler more modern package though, even if there's a need to maintain something that has deep-rooted legacy limitations. I have found that generators can be used to simplify code like this greatly, particularly when you use them as strategies that control the transitions of a state machine. If you design the state machine API correctly it essentially becomes a de-facto plugin API because you can always use higher-order strategies to extend any existing functionality. An immediate gain is that you end up with normal top-down control, and thus useful stack traces! And of course you can provide sync and async variants with a single implementation. And it becomes possible to create a visual state machine, devtools for resolvers essentially. This isn't a big deal when you have one resolver, but what we actually need is a huge family of related resolvers, so that it rapidly becomes useful to be able to explain at any given moment what a resolver is doing and WHY. |
@ljharb As @IanVS stated I don't want to add to the pressure and I know you are a busy guy doing a lot of things for free. As you've stated so many other things would be fixed (like eslint-plugin-import) if resolve supports ESM resolution. Given the many problems with build tooling around the big push towards ESM (especially when it comes to consuming ESM-only packages) it would be really great to finally fix this issue. Just trying to be a bit of a squeaky wheel without being too annoying... |
Would be great to have this implemented since it blocks eslint-import-resolver-node from resolving packages that do not configure the |
Using eslint, I'm getting an `import/no-unresolved: Unable to resolve path to module 'vite-plugin-ruby'` when it clearly exists. Looking at [this ticket](browserify/resolve#222) I believe that is happening because there is no `main` key in the package.json. This updates the package.json to include this.
@SimenB I wonder if the most expedient solution would be to make |
Jest uses |
Hiya! 👋
We've discussed this briefly on Slack, but I thought an issue would be easier to keep track of.
I'm not sure what it would entail, so I'll just list out (i.e. do a brain dump of) the features I think would be needed.
resolve
will only ever return a single string, which is the absolute path to the resolved file. I think in addition to this, we'd need to know if the file should be interpreted as CJS or ESMresolve
taken an option what the caller would prefer? Should it be a static option in, or some callback?exports
field in package.jsonconditions
resolve
when implementing custom loaders or using the VM APIs.import()
supports it. It probably makes more sense for resolve to return URLs than an absolute path for ESM?There is also a flagged API in Node for this,
import.meta.resolve
: nodejs/node#31032. Not sure if we should care too much about it, though?I think that covers it, but you know way more about this subject than I do, so feel free to either close this to open up your own, or edit this OP as you see fit 👍
For background, Jest uses
resolve
as the default implementation injest-resolve
, although the user can swap it out. I'm currently working on support for ESM natively in Jest, and while we have a version today that sorta works, it's not a compliant implementation. Most of the (known) issues are due to resolution logic. I'd be happy to help implement support here inresolve
.The text was updated successfully, but these errors were encountered: