-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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 for Import Maps #49443
Comments
Since import maps seem far from being finalized or standardized in browsers, it feels like it’d be best to confine it to userland loaders for the time being. |
@ljharb Added an open question at the end with my concern:
|
I don’t think it would supersede it - they’d just get to provide an import map instead of their current approach, whenever it became available. Perhaps @arcanis can weigh in here. |
I assume that That being said: Having a well-supported loader that uses an import map would be neat to have. But I agree with @ljharb that at this point it doesn't really belong into node core. There are some pretty major open questions with import maps in node as well. E.g. your example lays out relative file URLs. One major development in more recent package layouts was that the files do not actually live in |
"Instead" is superseding it. Users will couple to what the package managers release, and it will not be a simple matter to just replace it. This could be a large hurdle to overcome in the long run.
I agree we would want strong buy in from the package managers. I presented a PoC of workspaces built on top of this to @ruyadorno today and he is going to show it internally at npm. I would love to also hear what @arcanis has to say on this.
This is part of the problem I hope we could solve here. If node core supported a shared format we wouldn't fragment the ecosystem with package managers providing their own loaders. It is much better IMO to have them build to a shared spec the runtime understands.
Import maps specifically solve this problem. They can point anywhere on the filesystem. In the workspace example I have (not oss yet) I am working on moving them into a shared workspace cache.
In the global shared cache scenario it would not be portable. The example I showed using relative urls to make it portable, but my workspace PoC uses full paths. I think it is reasonable to support both. |
Amazing to see this work, thanks @wesleytodd for investigating. @ljharb would you disagreement over stability be mitigated by flagging such an implementation. Personally I see no harm in experimentation in core, rather than pushing out experimentation here entirely to userland. It shows a willingness for core to cooperate with projects exploring this space over forcing completely separate approached entirely leading to the fragmentation @wesleytodd mentions. It may well work out that we find that projects do need their own full import maps implementations, in which case Node.js can always deprecate such an experimental flag based for that outcome. As @jkrems has touched on, one of the issues here is that import maps as a replacement for the entire resolution system is tricky due to these details. For example, with conditional exports, the same package can resolve different depending on environment information. Import maps don't have this same functionality and likely won't... rather they can be considered an output of an environment function at some level - an import map specific to the browser is different to an import map specific to Node.js. In this way I prefer to think think of import maps somewhat like "ephemeral views" into the project, representing a caching / precomputation of the resolution operation. They have a locking property, but that should not be confused with lock files, which are about install reproducibility, something that is clearly a superset of the features import maps provide. Thinking about the user features, I see these as goals of import maps in Node.js, when they can be allowed to be complementary to the Node.js resolver:
I do see having import maps as being the default and only form of resolution in Node.js as a non-goal here though. Rather an approach where the import map can allow intercepting the resolution and then having that work with userland approaches to explore the above space seems most beneficial to me. |
Thanks for responding on twitter and sending me down this path :)
I think this is also solvable inside import maps. The spec doesn't really specify what the scope can be used for, so I see no reason that information cannot be encoded into the scope mapped from the conditional exports. Either that or the tooling would generate a separate environment import map (as you mention).
In the PoC I have not shared yet I actually use npm (
Agreed, and thus my example falling back to the existing file system based resolution. I think this would be a very important key feature to keep. |
Does
I'm interested in how large the generated import maps would be. The |
That is really how I am testing it today. If you check the
No. It only lists packages then falls back to file system imports for the rest. Although that was a decision I made, which you could change for many interesting reasons (as Guy mentioned, mocking).
My guess? Never. That said we should test it. Also, it would be really simple to prune the import maps based on static analysis if the app entry point does not load anything from that module. It would also make sense that the package managers would generate a
I also am guessing here, but not much. I will try this out on the largest projects I can find at work and report back on the size. Also, we can easily optimize the the memory footprint from the import map format to something better at runtime then just throw away the json format. |
Ah not sure how I missed the example link. I'll check that out as soon as I have a chance.
If the import-map is has module granularity (not listing every file within) then this greatly reduces my performance/memory concerns.
Static analysis sounds difficult if possible, especially on the development side. I'm thinking of babel plugins as a perfect example, without specific knowledge of babel you cannot know that |
@guybedford flagging it is better, for sure, but i take it as a given that it'd start out flagged. My main concern is that unless import maps stabilizes, is standardized in browsers, and also is usable for node without doing backflips, then i'd hope node core would never ship any form of import maps - put more positively, when those things happen, node core would obviously ship import maps! Given the large uncertainty, it seems premature to me to even signal a possible future core implementation when there may be none at all. |
The problem behind disjoint workspaces isn't in term of resolution - that's fairly simple to implement - but rather developer experience. For example, if you run ¹ It's a rhetorical question; there are ways, but they are all less good that just telling our users to use
I don't think it would. It's not even clear whether import maps would have all the features we would need. For example, Yarn is able to throw semantic exception when a package isn't reachable, explaining why the package cannot be found. It's quite harder for import maps, which don't have package-level semantics - just folder-level semantics:
I think my main question is: what do you think this would improve? I know you mentioned you would find it cleaner, but that's subjective. I personally find a JS API clearer than a file format, for example. So in concrete terms, what would it bring to the table?
Overall I'm just not convinced this is the most impactful place to put energy on. I'd rather talk about making the |
So, we already have this feature of static resolution mappings to some extent using https://nodejs.org/api/policy.html#policy_dependency_redirection ; in all honesty, even though I wrote the feature and have spent some time tuning performance, the benchmarks there are not pleasant when you run them on medium sized apps particularly regarding startup time. I think we should be more critical about the benefits here as it is not a sure win in terms of performance to merely have static resolution. If we could also address the other concerns about the limitations of only having data for resolution and adding additional meta-data that seems fine, but starts to seem to just be working towards a different policy file. |
Where can we look to help provide more certainty? Is it just a matter of "we cannot do it unless the browsers ship it first"? I obviously do not want to repeat the mistakes of the past, but this seems like a hard line to take long term.
I think tooling DX is a separate concern. I think the goal of this as a core api would be to enable experimentation in user land on top for what DX should look like. The goal of this proposal is to focus on if this is a good idea for inclusion into core.
Two concepts which are exclusive to yarn, made up by yarn, and so not a solution in this context unless you seek to standardize them. This only continues to widen the chasm between package managers, and increase the barriers to entry for another package manager.
IMO, this could be a good addition to core, It bridges the gap and brings the whole community up together. If there are hints or metadata which pacakge managers can give to the runtime to improve in this way it would be great! We should look for a loader api which enables this, and I believe that import maps could be that api.
I am fine with it being a js api. My internal workspace PoC is that, it follows more closely the approach in PnP, but uses an import map as the structured input format.
Completely agree! This paragraph this sentence is in is exactly my goal here.
Do you mean this spec? https://whatwg.github.io/loader/
This is close but not equivalent right? Also I think the use case of security concerns remapping modules is a bit different scope than import maps. I actually think that import maps with the features in the policy api would be a better way to implement what is being done because it would be more generic and more approachable by users.
Do you have links for this? I would love to look at them. I admit that I have done no benchmarking in this regard, and it for sure would have to be done. |
@wesleytodd see stuff like #29527 which would add a benchmark to node specifically about that feature. |
I would be fine with creating a new flag to experiment on supporting import maps. If it becomes a standard, Node should support it. I agree that there are questions of how useful import maps really would be to Node. The new |
It is a bit different. The goal of an import map is that tooling can do creative things with where the modules live, which is different than what the exports. Right?
Strong 👍 on this! I think that waiting on browser vendors is a passive approach. Having a flagged feature which gets good usage is a strong signal to the browsers that this implementation works. It also gives us a better seat at the table in the discussions because we will have experience building and maintaining it. |
I'd be against stating just because something is a standard it is good to land it everywhere. The implications of import maps and the integrations across environments does not seem to be well discussed. If we saw some interest in import maps beyond the current browser heavy path forward to address things like those in this issue such as having more data, composition on a package level, etc. I'd be more interested in that specific feature. As it stands things, Node is not beholden to follow any given standard it does not feel is a good fit for the runtime; and, as it stands I do not see import maps as being something that is well suited towards being merged. Though, I would state I am generally biased against WHATWG controlled features in general these days and more so biased against them the more interactions I've had over the years so it isn't like I'm a neutral party here. |
More interest from whom?
I was hoping to make that case, are there specific things you think I could expand upon above which would better make the case for you? I have mentioned that I have a PoC of one key feature it has that current solutions do not provide. If I provided some more use case examples than my very simple gist would that help?
People matter, and if you don't have faith in their process is there something we could do to move the specification into a better home? |
People wanting to implement import maps in other environments <-> the import maps proponents. As of yet, we have some interactions about how other environments may use what the web wishes to use, but not about what the web can allow to let other environments have their own use cases.
One of the big things is composition being discussed at length. E.G. Placing something under Other data such as integrity strings would also be of benefit to discuss. I don't think the path resolution is the only thing that will be used when doing imports. In the future, browsers are also looking at requiring module attributes and those likely also need to be discussed. I think providing use cases that elaborate on comparisons with existing Node features and explanation of why import maps are a better fit than a Node specific alternative would be helpful for these kinds of things.
This is part of the issue, things like RFCs have sometimes been taken over and superseded by WHATWG claiming to be the source of truth rather than the original RFC. I doubt moving the specification would prevent such behavior in the future. |
I have read through most of the conversation on this. I agree the direction that conversation takes could effect our implementation. That said, I think the only thing which makes sense for node here is to have one import map and no composition. In the browser this doesn't work, and I understand that. Is there a reason we could not implement only a subset and not support composition ever in the future? Having a dep define an import map in node has issues, it would have large security concerns and usability concerns.
Agreed! Digging into your policy api stuff I really think there is overlap here we would want to resolve.
Is the alternative to write a separate rfc with the same work? I really prefer to avoid conflicts or politics where possible lol. What could we do to get the equivalent functionality while avoiding this issue? |
I do not have an answer to that question due to the above behavior of overtaking anything we might propose as separate. |
FYI, Deno supports import maps: https://deno.land/std/manual.md#import-maps |
Since it has been brought up recently, my current take is: It doesn't sound like there have been any signals that npm and/or yarn would adopt it. And without that, the case doesn't seem all too convincing right now. Even on the browser side, Chrome seems to be the only party more or less actively investing into import maps. There's also some pretty fundamental questions:
It feels like node support for import maps is fairly far down the list of "things that block people from using import maps". So I'm not sure it's a good time to invest into adding it to node.
Thanks, that's good to know! But it's worth noting that they're running into the exact problem that I'm afraid of for node: They added support almost a year ago and there's still no good story for actually using import maps (see denoland/deno#3585). I think that's a good example of why I think that native support in node isn't the most important step if the goal is to make import maps in node viable. |
I can work on this. That said, I do not feel that this should be part of the acceptance criteria. Other tooling can fill in here if they are not interested.
Yes. My proof of concept, while using a ESM loader works on CommonJS.
I think the tool generating the map needs to know the target. I haven't fully thought through this design, and we would for sure want a clear story on this. I think avoiding making any changes to the spec without also working with them to land those changes in the spec is a bad idea, but I think it just works as spec'd today.
I think before PnP and tink like functionality, which heavily overlap with this proposal, gain large scale adoption is the perfect time. Otherwise we are just fighting against the current.
It sounds like the issue they have is specifying the flag. With a loader we have the same problem, but my proposal is to give a standard location which will be loaded by default. Does this not circumvent the issues described in that deno issue? Also separately they are coping with the design decision to import from URLs at runtime. Things which I strongly advise against, and something which (from my position at Netflix) will be a huge concern if Node were to adopt. |
Right now we don't support custom loader hooks in CommonJS. And if this becomes a built-in feature in node, I assume we'd have to implement it in the actual require loader. There's no call from the require loader into the import loader for resolution. E.g. CommonJS does recursive searches that aren't really how import maps work. There'd have to be some statement on expectations for this.
I'm pretty sure it doesn't, the spec removed fallbacks afaik. So I don't think we could generate a valid import map with conditional resolution. Happy to be proven wrong but that's my current understanding.
Conditional exports do allow for different conditions at runtime. E.g.
Since both of them have working implementations/designs that support CommonJS fully - why do we think that import map support in node would change the adoption probability? I don't think end users care if they pass |
Sorry, I was unclear here. My POC uses the existing
I was not aware the specification had an opinion here. Do you have links on this?
I will look into how this could work. My first naive idea is that the
I think the fact that they both have low popularity solutions for this is a sign that a common shared core belongs in node. I think that both should be able to build on top of the same base, instead of the fragmentation which is happening currently. If there is another better proposal for a common core, I am fine going in that direction. Maybe I could see package installers providing loaders as an option, but that is just even stronger fragmentation. With the current direction it seems to be leading to incompatible ecosystems, I would hate to see one of the strongest parts of the node ecosystem broken because we fail to set a shared direction on this. I would rather have an imperfect solution which aligns people with compatibility in mind, than a perfect solution in 5 years when everything has fragmented off. |
This comment has been minimized.
This comment has been minimized.
FYi, It will be sweet to have package management tooling that works the same way regardless if writing for server or client, regardless of which runtime or browser. |
It’s at the top of the loaders to-do list. Pull requests are welcome. If you start working on a branch, please let us know at https://openjs-foundation.slack.com/archives/C053UCCP940. |
There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment. For more information on how the project manages feature requests, please consult the feature request management document. |
This is still relevant |
Hey @joeljeske, when you drop into a thread like this please try to provide a bit more context on why you are asking and what your goal is. As it stands, I would likely mark your comment as off-topic or spam even. The work is still underway over in #50590 if that is what you mean. |
@wesleytodd I believe @joeljeske might have just wanted to make sure the issue remains open. I'll go ahead and remove the stale label. |
FYI, I asked in the OpenJS Slack about this bot. I am happy to open an issue (or maybe even a PR if it is a fast enough change) to improve the language. Honestly I am not even sure if this issue should remain open since there is a PR now, but if I am going to close it I would like to also improve the message that "it is unlikely to be implemented" which is not correct and too overbearing for a both to make that claim. |
Yes, thank you. I was wanting to deactivate the stale bot. I should have also made my own language more clear. My apologies. Thank you for your continued work on this effort! |
There has been no activity on this feature request for 5 months. To help maintain relevant open issues, please add the
never-stale
|
I've added the never stale here, as I don't know of any resistance to implementing this feature in Node.js, other than just getting the PR to a mergeable state. Happy to be corrected on this though if there are other perspectives here. |
I have had a few conversations recently about working with a few other folks who have time and are able to help. It is nearly over the finish line, and if I had any dedicated time to work on it since last year I would have loved to finish it. If no one makes a serious attempt to work with me on this before Jan, I hope to have that time to finish it up then. So not sure what stale does, but I would prefer if it was not closed. |
Would the idea be that conditions are resolved at package installation time? Or would we use an "extended" import map that supports conditions and potentially also wildcard patterns to reduce the size / IO required to generate? |
Can you clarify your question? Are you suggesting non-standard additions? If so, I think we should keep that discussion separate from this PR which is intended as only the base level of support for the whatwg spec. If not, which is entirely likely since I am not clear on the question, can you help me better understand your goals? |
@wesleytodd Basically, if I am a package manager and want to generate an import map on install, I would have to handle packages that conditionally export different things. For example: {
"name": "pkg",
"type": "module",
"exports": {
"node-addons": "./addon.node",
"default": "./slow.js"
}
} In the import map, there can only be one entry for Just wanted to bring this up as a "what does it mean to support import maps", in terms of support for existing packages. The answer could absolutely be "this becomes a package manager/outside tooling problem and node stops caring about conditionals". And this doesn't have to stop a PR to land raw browser-style import maps. |
I was hoping to start the conversation around getting Import Map support into core. I figured starting here with a proposal was better than starting right off with a PR.
What Are Import Maps
The proposal was mainly targeted at browsers where URL imports have many downsides without bare specifier support. It enables a standardized map file format to be used when resolving import specifiers, giving developers more control of the behavior of import statements.
Here is an example of the format:
Proposal link: https://github.com/WICG/import-maps
Why should they be supported in Node.js?
Currently we use the structure of the filesystem and a resolution algorithm to map import specifiers to files. These files are typically installed by a package manager in a structure which the Node.js resolution algorithm knows how to resolve. This has some drawbacks:
node_modules
at your filesystem root for exampleIf we had import maps we could circumvent most of this. Package mangers can generate an import map, enabling perf improvements and simplicity in their implementations. It increases startup performance of apps because they don't have to do as much filesystem access.
One interesting example of what import maps enables is filesystem structure independent workspaces (think yarn workspaces but without the requirement of the modules being under the same root directory).
Implementation
As an example implementation I created this gist:
https://gist.github.com/wesleytodd/4399b2351c59438db19a8ffb1f3fcdca
To run it:
This uses an experimental loader which loads an
importmap.json
and falls back to the original filesystem lookup if it fails. I am also pretty sure this very naive implementation is missing edge cases, but in these simple cases it appears to work.Obviously for support in core we would not use a loader, but I think the rest would be similar. Open questions I have are:
Where would node find the
importmap.json
?My first idea is at
node_modules/importmap.json
. I think this would be expected behavior from users if their package managers started writing this file.How would users override the
importmap.json
?For this I think a reasonable approach would be a cli flag (
--importmap
) and a package.json ("importmap": "./importmap.json"
) field.Do we want to wait for browsers to standardize import maps?
#49443
I think that this is a very valid concern. Is there a way to help push that forward? The benefits are pretty large in node IMO, and it would superseded all the work currently being done on yarn pnp and npm 8. If we let those go forward but then this spec lands it might be even worse for shifting the community.
Could we release it as flagged & experimental until browsers standardize? This would mean users could opt in, but as it changed we could follow along.
Thoughts? Next steps?
The text was updated successfully, but these errors were encountered: