-
Notifications
You must be signed in to change notification settings - Fork 69
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
Not working with native ES modules #59
Comments
@cullylarson Ran into the same issue. It appears that the new esm code isn't running the _resolveFilename which is the core of this library. Based on the docs it looks as though they are moving off of this library's hack and onto a standard feature: https://github.com/nodejs/node/blob/master/doc/api/esm.md#experimental-loader-hooks It's still experimental though. I rewrote and reduced a lot based on known things within my library but this code is working for me:
Then: |
@jdt3969 Thanks for sharing. I'll give it a try on my next node project. |
Very interesting ! So if I understand correctly, you must just provide a .js file in a --loader flag ; and that .js file be a module that exports a So it'd be very easy to do a pull request (wink wink, nudge nudge) that exports a neat little module that wraps
@cullylarson @jdt3969 interested in doing a PR ? @ilearnio any additional thoughts ? |
@Kehrlann Thanks for the kind invitation to do a PR. I really appreciate the way you presented it. I'm near a deadline on a project right now and about to start another, otherwise I would take you up on the offer. |
You guys think we can do this without using the |
Not yet solved natively without execution flags? |
@kirkouimet @eouia the node ems documentation linked above suggests no, the flag is required. |
@jdt3969's above solution worked for me after I realised it was looking for a property in package.json called I tidied up the above code and added some pre-computing of values that can't change during execution: gist EDIT: This was due to an error in my own source-code, which node ESM erroneously reported as coming from alias-loader.mjs |
This is really interesting and the gist looks promising @jshado1. But I'm wondering if this functionality can already be used when publishing a module to npm. I guess not, because how am I supposed to specify |
@TimDaub Thanks!
You probably shouldn't as loaders are imminently changing.
I'm not sure I understand what you mean—are you talking about where your module would itself depends on the loader, or someone consuming your module would need to consume it via |
Worked for me as well, though it's important to note that you now need to use |
Later. Imagine I published a package that relied on starting node with
That makes it unusable for npm packages until a version of node is out that removes the flag. |
I understand that this is kind of an issue with Node itself, but it would be nice if the README included a warning about this package not being compatible with native ES modules. I just spent an hour questioning my sanity only to find out that the feature I've been hopelessly debugging was never intended to work at all. |
@szydlovski good point, would you want to submit a PR? |
@Kehrlann Sure, here's a proposal |
@TimDaub "unusable" does not seem accurate at all, and such CLI flags are quite common in many, many libraries. Support for a non-CLI option has been briefly discussed as a possibility in future, but if it happens, it would likely be quite a bit down the road. How loaders in Node.js will (almost surely) work once loader chaining is supported would be something like $> node --loader https-loader --loader typescript-loader ./your-app.ts |
Having to mention in the installation instruction on the library level that the using application should be initiated with a loader flag makes it unusabe. It's fragile declaring this information in a readme. On the library level, the loader dependency should be described in the package.json and any using higher level package should interpret the lower libraries' loaders through the package.json. But I guess that's out of scope for module-alias. |
That sounds a bit dramatic. It's called a README (in screaming capitals) for a reason…. This is classic RTFM. Module alias is obsolete now anyway with import mapping. |
This problem has been plaguing me for years because I want to write good quality code that is shared between node & browser.
'nesm.js'
'nesm' shell script
|
You should have said that in |
@euberdeveloper I did #59 (comment) you can also see it on line 9 of the code. You're welcome btw. |
In this code there is a problem with Windows paths.
I made an npm module using this code that fixes also that problem, check it out here: esm-module-alias |
Heads up: the node loader API function signatures have changed, and the 2nd argument is an (optional) object (not parentURL). Supplying an argument of invalid type will trigger an exception. Please see the Node.js docs for current signatures. |
Could you please link me the signature in the doc? |
https://nodejs.org/api/esm.html#hooks I'm not sure what version(s) of Node.js the change was backported to, but at the very least, my code from ~2 years ago and what you included in your npm package will not work in the v18 (LTS) or v19 (current) of node. |
My repo does some tests with also versions of nodejs 18 and 19, and it seems to work |
For anyone else coming across this looking to support wildcard imports replace:
With
|
I'm moving a project over to using node's native ES modules (enabled with the
--experimental-modules
flag). After updating my code, module-alias is no longer working. I tried adding this to the root of my app (this is the method I was using before transitioning to esm):I tried changing it to:
I tried requiring when starting the server:
The first aliased import in my app is this:
I'm getting this error from it:
The relevant lines in my package.json are:
I'm starting the app like this:
module-alias worked fine using CommonJS. The only change I made to the code was changing
require
s toimport
s.The text was updated successfully, but these errors were encountered: