nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
108.07k stars 29.83k forks source link

Support for Import Maps #49443

Open wesleytodd opened 4 years ago

wesleytodd commented 4 years ago

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

This proposal allows control over what URLs get fetched by JavaScript import statements and import() expressions. This allows "bare import specifiers", such as import moment from "moment", to work.

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:

{
  "imports": {
    "todayis": "node_modules/todayis/index.js"
  },
  "scopes": {
    "node_modules/todayis": {
      "english-days": "node_modules/english-days/index.json"
    }
  }
}

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:

  1. It is slow
  2. It can lead to unexpectedly resolving modules
    • if you have a node_modules at your filesystem root for example
    • if your package manager has hoisted a transitive dep and you access it from the top level
  3. Tools are required to implement copies of the Node.js resolution logic
  4. Users and package managers jump through hoops to try and make the filesystem performant

If 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:

$ git clone git@gist.github.com:4399b2351c59438db19a8ffb1f3fcdca.git hello-today
$ cd hello-today
$ npm 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?

https://github.com/nodejs/node/issues/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?

ljharb commented 4 years ago

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.

wesleytodd commented 4 years ago

@ljharb Added an open question at the end with my concern:

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.

ljharb commented 4 years ago

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.

jkrems commented 4 years ago

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.

I assume that import-map.json would be written by package managers (if they choose to support it). Both yarn and npm have already somewhat committed to a public interface for their resolution structure. Without strong signals from their side that they'd realistically move to a shared metadata format, I don't see them making the switch. pnp.js would stick around anyhow. And require-style resolution would still be needed for backwards compatibility. So they would need two files instead of the one they already have.

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 node_modules/ (if possible). And in that world, a stable (version controlled) import-map.json can't (?) really use actual file URLs afaict.

wesleytodd commented 4 years ago

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.

"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.

Both yarn and npm have already somewhat committed to a public interface for their resolution structure. Without strong signals from their side that they'd realistically move to a shared metadata format, I don't see them making the switch.

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.

pnp.js would stick around anyhow.

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.

One major development in more recent package layouts was that the files do not actually live in node_modules/ (if possible).

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.

And in that world, a stable (version controlled) import-map.json can't (?) really use actual file URLs afaict.

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.

guybedford commented 4 years ago

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.

wesleytodd commented 4 years ago

Amazing to see this work, thanks @wesleytodd for investigating.

Thanks for responding on twitter and sending me down this path :)

For example, with conditional exports, the same package can resolve different depending on environment information.

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).

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.

In the PoC I have not shared yet I actually use npm (@npmcli/arborist technically) to generate the tree and then the import map. This means that it respects the lock file, and will reproduce the same import map from the same lockfile.

I do see having import maps as being the default and only form of resolution in Node.js as a non-goal here though.

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.

coreyfarrell commented 4 years ago

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.

Does --experimental-loader make this possible already? For testers npm i wesleytodd/import-map-loader would be easier than rebuilding node.js.

In the PoC I have not shared yet I actually use npm to generate the tree and then the import map. This means that it respects the lock file, and will reproduce the same import map from the lockfile.

I'm interested in how large the generated import maps would be. The node_modules of one of my current projects contains 576 modules and 6355 .js files. Does every .js file get listed in the import-map? At what point does the load/parse of a large import-map exceed the current resolver overhead for loading only a couple files from node_modules? I'm also interested in the potential memory overhead of keeping a large import map loaded.

wesleytodd commented 4 years ago

Does --experimental-loader make this possible already? For testers npm i wesleytodd/import-map-loader would be easier than rebuilding node.js.

That is really how I am testing it today. If you check the test script in the package json of the example you see it passes --experimental-modules --loader ./loader.mjs

Does every .js file get listed in the import-map?

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).

At what point does the load/parse of a large import-map exceed the current resolver overhead for loading only a couple files from node_modules?

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 --production which would only be the production dependencies.

I'm also interested in the potential memory overhead of keeping a large import map loaded.

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.

coreyfarrell commented 4 years ago

Does --experimental-loader make this possible already? For testers npm i wesleytodd/import-map-loader would be easier than rebuilding node.js.

That is really how I am testing it today. If you check the test script in the package json of the example you see it passes --experimental-modules --loader ./loader.mjs

Ah not sure how I missed the example link. I'll check that out as soon as I have a chance.

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).

If the import-map is has module granularity (not listing every file within) then this greatly reduces my performance/memory concerns.

At what point does the load/parse of a large import-map exceed the current resolver overhead for loading only a couple files from node_modules?

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 --production which would only be the production dependencies.

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 {presets: ['@babel/env']} in babel.config.cjs means that @babel/preset-env is loaded.

ljharb commented 4 years ago

@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.

arcanis commented 4 years ago

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).

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 yarn add foo into the disjoint workspace, how does Yarn knows that it needs to update the lockfile from a completely different folder¹?

¹ It's a rhetorical question; there are ways, but they are all less good that just telling our users to use link: or portal: to add their disjoint workspaces as external dependencies. Not everything has to be in the core 🙂

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.

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:

Something that got detected as your top-level application (because it doesn't seem to belong to any package) tried to access a peer dependency; this isn't allowed as the peer dependency cannot be provided by any parent package

Required package: react (via "react")
Required by: frontend/app.js

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.

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 fs module extendable in a proper way, for example.

bmeck commented 4 years ago

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.

wesleytodd commented 4 years ago

Given the large uncertainty, it seems premature to me to even signal a possible future core implementation when there may be none at all.

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.

but rather developer experience

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.

they are all less good that just telling our users to use link: or portal:

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.

For example, Yarn is able to throw semantic exception when a package isn't reachable

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 personally find a JS API clearer than a file format, for example.

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.

This to me signals more a lack of good API that the need for another universal format.

Completely agree! This paragraph this sentence is in is exactly my goal here.

loaders are able to do everything the import maps can do, plus more.

Do you mean this spec? https://whatwg.github.io/loader/

we already have this feature of static resolution mappings

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.

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.

the benchmarks there are not pleasant when you run them on medium sized apps particularly regarding startup time

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.

bmeck commented 4 years ago

@wesleytodd see stuff like https://github.com/nodejs/node/pull/29527 which would add a benchmark to node specifically about that feature.

GeoffreyBooth commented 4 years ago

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 package.json "exports" implements a lot of the same functionality, with additional features; and we designed "exports" to be easily convertible by tools into import maps. That said, Node should absolutely support any standards; and we would be in a stronger position to influence the standard as it develops if we have our own flagged implementation for folks to compare against the browsers’ flagged implementations. If import maps never end up getting standardized, we can just remove ours.

wesleytodd commented 4 years ago

The new package.json "exports" implements a lot of the same functionality,

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?

we would be in a stronger position to influence the standard as it develops if we have our own flagged implementation

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.

bmeck commented 4 years ago

That said, Node should absolutely support any standards

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.

wesleytodd commented 4 years ago

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.

More interest from whom?

As it stands things, Node is not beholden to follow any given standard it does not feel is a good fit for the runtime

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?

biased against WHATWG controlled features

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?

bmeck commented 4 years ago

More interest from whom?

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.

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?

One of the big things is composition being discussed at length. E.G. Placing something under node_modules/ means that the location in which a program is being run affects its import map. Since multiple node_modules/ folders may exist (or none) this means discussion of how multiple import maps needs to take place and likely cannot be put off like in the browser.

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.

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?

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.

wesleytodd commented 4 years ago

One of the big things is composition being discussed at length

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.

Other data such as integrity strings would also be of benefit to discuss

Agreed! Digging into your policy api stuff I really think there is overlap here we would want to resolve.

I doubt moving the specification would prevent such behavior in the future.

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?

bmeck commented 4 years ago

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.

GeoffreyBooth commented 4 years ago

FYI, Deno supports import maps: https://deno.land/std/manual.md#import-maps

jkrems commented 4 years ago

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:

  1. Can we support import maps in CommonJS? If not, are we okay with that?
  2. How would import maps work with conditional exports? Would we support having multiple import maps? Would we extend the import map into something that wouldn't actually match the official spec?

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.

FYI, Deno supports import maps

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 https://github.com/denoland/deno/issues/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.

wesleytodd commented 4 years ago

It doesn't sound like there have been any signals that npm and/or yarn would adopt it

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.

Can we support import maps in CommonJS? If not, are we okay with that?

Yes. My proof of concept, while using a ESM loader works on CommonJS.

How would import maps work with conditional exports?

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.

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.

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.

They added support almost a year ago and there's still no good story for actually using import maps

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.

jkrems commented 4 years ago

Yes. My proof of concept, while using a ESM loader works on CommonJS.

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.

but I think it just works as spec'd today.

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.

I think the tool generating the map needs to know the target.

Conditional exports do allow for different conditions at runtime. E.g. import vs. require or a speculative private/internal condition.

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.

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 --import-map=tink.modulemap.json or -r tink. Especially if PnP/tink end up as loaders and may even be specified in package.json one day.

wesleytodd commented 4 years ago

Right now we don't support custom loader hooks in CommonJS.

Sorry, I was unclear here. My POC uses the existing --loader feature, not loader hooks. I agree we would need support in both contexts.

I'm pretty sure it doesn't, the spec removed fallbacks afaik.

I was not aware the specification had an opinion here. Do you have links on this?

Conditional exports do allow for different conditions at runtime. E.g. import vs. require or a speculative private/internal condition.

I will look into how this could work. My first naive idea is that the scope is the right place for this logic to live. If the loader did the same logic it would do based on the package.json field but inside the import map, seems like a scope could look like /path/to/scope::import. Seems like that is inline with the intent of the spec no?

why do we think that import map support in node would change the adoption probability?

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.

jkrems commented 4 years ago

My POC uses the existing --loader feature, not loader hooks.

Sorry, when I say "loader hooks" I mean the --loader flag which has an unfortunate name (since it doesn't allow overwriting the actual loader, only providing hooks into the loader).

I was not aware the specification had an opinion here.

The spec only allows for one resolution of a given specifier per scope. The following CommonJS file wouldn't work with that restriction, even if every single file gets its own scope:

require('x'); // needs to use require condition
import('x'); // needs to use import condition

seems like a scope could look like /path/to/scope::import. Seems like that is inline with the intent of the spec no?

Having a postfix scope would only work if you'd leak the scope into the module URLs (so they match prefix) or do some rewriting of the data that isn't part of the import map spec algorithm. I don't think that's really implementing the spec anymore.

I think the fact that they both have low popularity solutions for this is a sign that a common shared core belongs in node.

We have some fairly specific concern from PnP about adopting this format and how likely it is to match their requirements. Especially once you throw in CommonJS support and modules that have direct disk access (e.g. to load data files), import maps haven't really been compelling to them. Nowhere in their list did I hear "native support in node" as a primary concern. So it feels awkward to claim that the lack of popularity is because of the lack of native support in node for import maps.

arcanis commented 4 years ago

I think the fact that they both have low popularity solutions for this is a sign that a common shared core belongs in node.

The adoption rate for PnP has nothing to do with the file format, but rather the inertia caused by invalid package manifests and node_modules direct accesses. Whether it's PnP or import maps, the results will absolutely be the same. If you want to help making alternate resolution strategies popular, help fix the packages that make those assumptions.

wesleytodd commented 4 years ago

Sorry, when I say "loader hooks" I mean the --loader flag which has an unfortunate name (since it doesn't allow overwriting the actual loader, only providing hooks into the loader).

Maybe I am confused here, my POC is using CommonJS. I assumed by your question you were saying that it would not, but it is using the tried and true "override module._resolveFilename". Which is different than the loader hooks documented here: https://nodejs.org/api/esm.html#esm_experimental_loaders

Having a postfix scope would only work if you'd leak the scope into the module URLs

Can't this be an implementation detail of require and import? The specifier can remain the same, but depending on the context it can use a different lookup in the scope portion of the import map.

I don't think that's really implementing the spec anymore.

Yeah, this is a leaky abstraction for sure. Maybe we could present this use case on the spec repo and get feedback?

So it feels awkward to claim that the lack of popularity is because of the lack of native support in node for import maps.

I did not claim this. I said that it is a sign that need exists. These are two very different issues. the fact that they are currently low popularity means we have a chance to improve the story before they gain momentum.

If you want to help making alternate resolution strategies popular, help fix the packages that make those assumptions.

IMO, this is a non-goal. If we can give tool implementers better low level tools which allow for the inherent sloppiness in the existing ecosystem, it is much better than trying to "fix it". I do not see import maps being a "fix" in any way for this, but I have not seen any unsolvable blockers either.

jkrems commented 4 years ago

Maybe I am confused here, my POC is using CommonJS.

Sorry, I assumed it was using the hooks because of --loader. So your POC would also work as just a pre-loaded file (--require ./import-map-cjs.js), assuming it was written as CJS itself. I think it demonstrates not much more than "you could do some lookups in an import map during resolution" since it doesn't actually provide require-style resolution, especially for sub-paths. And because you fall back to current file system lookups (which you kind of have to..?), it won't actually prevent bad use of accidental peers etc..

the fact that they are currently low popularity means we have a chance to improve the story before they gain momentum.

Indeed! It's the reason why we wanted exports in package.json and have pushed back hard against require-style file extension guessing in ESM. This means that today it should be almost possible to generate an import map and make an application work, given a small custom loader to integrate it.

But only if the application uses native modules exclusively. Because once CJS enters the scene, all bets are off. An import map generated just from package meta data, without tracing all imports, would be impractically large if it supports CJS lookups as well. At least according to my own estimates and from every other estimate I'm aware of.

Adding a feature to node that is useful for applications that are 100% native modules, including all deps, and that use a not-yet-existing tool to generate an import map, and that care enough about resolution speed (?) to accept the DX overhead of running a non-common stack - that just doesn't seem like a huge target audience.

And I can't imagine npm or yarn adopting import maps (given their explicit statements) just because there's native support in node. The piece of doing specifier resolution just isn't the hard part. So offering a built-in solution in node that likely wouldn't integrate well with any of their other features (e.g. a virtualized file system for deps for CJS resolution) - what exactly is the value proposition?

I'm excited about import maps as well. I think one day native support for import maps is somewhat likely to land in node. But on that road are much more valuable next steps than making node parse the current JSON files imo. At best it would be dead code that no userland code could reasonably leverage yet. At worst it would deviate from what ends up in browsers and gets removed in a year or two after causing a bunch of churn in the codebase.

wesleytodd commented 4 years ago

since it doesn't actually provide require-style resolution, especially for sub-paths

But it does. It falls back to the existing file system resolution.

it won't actually prevent bad use of accidental peers etc..

Again, I think this should not be a goal. If it is an outcome that folks can use an import map to prevent it, that is fine, but making it a requirement seems out of scope to me.

But only if the application uses native modules exclusively.

I do not think this is true. If the import map implementation just respects the existing CommonJS patterns, then it works just fine.

An import map generated just from package meta data, without tracing all imports, would be impractically large

I would strongly urge we don't try for this outcome. Having the import map only generate what the package metadata provides if a great state. Let the rest follow what we have today with filesystem access. This is a good design decision IMO.

Adding a feature to node that is useful for applications that are 100% native modules, including all deps,

This is confusing to me. ES Modules are not required in any technical reason. What makes you think this is the case?

and that use a not-yet-existing tool to generate an import map,

Chicken and egg problem. I will build the tool if there is interest in moving forward with it.

and that care enough about resolution speed (?) to accept the DX overhead of running a non-common stack

This has much more than just resolution speed as a goal. And my goal here would be to make it not a "non-common stack". So I don't see this as a strong argument for not moving forward.

what exactly is the value proposition?

The value proposition is particularly beneficial to tools like npm. The virtualized fs is a response to today's module resolution. If we change the foundation it might change their mind about the right implementation.

At worst it would deviate from what ends up in browsers and gets removed in a year or two after causing a bunch of churn in the codebase.

This is IMO, the largest and most important concern brought up here. Since it has come up a few times in this thread I will go ahead and add links and more details about in in the OP.

jkrems commented 4 years ago

Chicken and egg problem. I will build the tool if there is interest in moving forward with it.

It's not though. A userland tool that generates import maps could easily also have a --loader and/or --require interface for applying it to the resolution. Yarn already did this today, with a more complex resolution data structure. So there's no chicken/egg here. Nothing stops progress today afaict. :)

The value proposition is particularly beneficial to tools like npm. The virtualized fs is a response to today's module resolution. If we change the foundation it might change their mind about the right implementation.

So far I haven't heard a strong argument why it would change their mind. If they still have to hit the disk anyhow, they have to implement the entire current solution. So import maps just mean that they have to implement what the already have and then in addition implement something for import maps.

When the domain expert (@arcanis) says he doesn't see the value proposition, I have a hard time taking in blind trust that there is a value proposition.

ljharb commented 4 years ago

s/the/a.

jkrems commented 4 years ago

s/the domain expert/the only domain expert who has given us feedback?

wesleytodd commented 4 years ago

So there's no chicken/egg here. Nothing stops progress today afaict. :)

Ok, I can see what you mean here. I was thinking about it differently than you were, but yeah I can get onboard with just building it more robustly as a stand-alone userland tool via --loader/--require

they have to implement the entire current solution

I am very convinced this is not the case. I built a simple example of workspaces on top of arborist and import maps, and I have team members who are working on abstracting away the entire filesystem portions of npm which could mean that the package manager only resolves the tree and hands it off. The more I have this conversation the more I am convinced I do need to build something to illustrate how this particular addition remove so much of the complexity we have today.

When the domain expert s/the/a. s/the domain expert/the only domain expert who has given us feedback?

I hesitate to even ping other domain exports on this thread 😛 (so far none have been). I have been having very productive conversations with folks at npm via other groups and slack channels. I will reach out to them and see if anyone is willing to join the conversation here.

Starcounter-Jack commented 4 years ago

Currently, it is very hard to write code for multiple targets (browsers, Node, Deno, Quokka). Some implementors insist that Ecmascript module-specifiers are hard-coded fixations on how to resolve the module rather than an identifiers specifying the module referred to. In my opinion, developers that are moving to ES6 modules are forced away from Node to things like Deno as the current ES6 module resolution functionality of Node is just to naive.

Why not avoid the hardships that will come from not influencing the way ES6 module resolution evolves? Node can and should take the lead on this. I think the import-maps spec, or any other spec, would bend to the input of Node. But for that to happen, there must be some input to begin with. If import-maps are not great, what is the suggestion from the Node community? The important aspect is reference indirection and not import-maps per se. There might be better ways to make Ecmascript module-specifiers into module specifiers rather than file paths/locators.

After all, the methods of CommonJs are not realistic in other environments such as the browser. Trial and error roundtrips or fixed relative locations are not okay in browserland. Resources might be packed in some resource that is sent already and therefore a module deep down cannot have an opinion on the location of the actual bytes containing the module.

BTW. It is not only Edge, Chrome, Deno and Firefox that are working on import-maps, the tools we are using are polyfilling import-maps on the server side for older browsers. Other tools are building them into the packaging process. Libraries such as lit-html are already written using bare import specifiers.

jkrems commented 4 years ago

It is not only Edge, Chrome, Deno and Firefox that are working on import-maps

Just to clarify: Has there been signal from Firefox? Last I heard there was no movement on the side of Firefox. Import maps haven't even officially begun the standardization process yet on the web side afaik.

Libraries such as lit-html are already written using bare import specifiers.

ESM in node does support bare import specifiers. It also supports remapping paths in a more expressive way than import maps allow today (using the exports field). Can you elaborate what kind of features import maps would add here?

joeldenning commented 4 years ago

Firefox's stance on import maps is being tracked at https://github.com/mozilla/standards-positions/issues/146. There have been no public signals yet that I am aware of. (Although I'm just a bystander here, and may have missed one)

Starcounter-Jack commented 4 years ago

The reason the world is moving is not that the spec is finished. It is that there is really no other spec and we cannot hard code the location of a module. How would I do that in a generic library? I have no idea on how the final package looks as provided by the user of the library in a browser. ES6 modules are not single target where the module can make assumptions about its host topology as in Node and CommonJs.

Chrome - check Edge - check Deno - check systemjs - check

Starcounter-Jack commented 4 years ago

Regarding Firefox

https://wpt.fyi/results/?sha=551f7310d5bb82b63f2ccfaed10dff43cf6c459a&label=pr_head

Starcounter-Jack commented 4 years ago

It would be more confidence inspiring if Node took the lead rather than issue an "experimental warning" on something that is already rapidly happening around it regardless. I would love for Node to stay relevant in the post ES5 era as it is a great product.

Starcounter-Jack commented 4 years ago

ESM in node does support bare import specifiers. It also supports remapping paths in a more expressive way than import maps allow today (using the exports field). Can you elaborate what kind of features import maps would add here?

I'm not sure it adds anything other than a way to resolve bare imports or other semantic imports in other hosts. I'm afraid that my ignorance might be the problem here, because I'm was not aware of the stance that the export field would be the competing solution.

My point on bare imports and import maps was merely the fact that import maps are the only thing that allows bare imports in (some) browsers. I.e. import maps is the only thing that allows this to function the same in Deno, systemjs and in Chromium:

import { $ } from "jquery"

bmeck commented 4 years ago

So, we have --loader and policies both of which might be suitable alternatives if the desire is just to redirect specifiers depending on your constraints. If the desire is to use exactly the same format as import maps I do not believe we currently are looking into that but a PR and explanation of how it works regarding those 2 other features of Node would seem apt.

Starcounter-Jack commented 4 years ago

So, we have --loader and [policies]

It looks to me that Loader hooks is indeed an enabling technology to build something on top of but hardly something recommended to users as an turn-key alternative to import maps.

bmeck commented 4 years ago

@Starcounter-Jack I do not think loaders are a simple solution but policies might be, they have some different compromises in design choices though especially regarding granularity which might be less pleasing to write manually.

Starcounter-Jack commented 4 years ago

I do not believe we currently are looking into that but a PR and explanation of how it works regarding those 2 other features of Node would seem apt.

Fair point.

bmeck commented 4 years ago

Either way, discussing how adding import maps would integrate with those features is likely required.

Starcounter-Jack commented 4 years ago

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.

The only downside to that is that it is hard to influence a spec if you wait until it is already implemented and standardized.

wesleytodd commented 4 years ago

Obviously as the original author here, I agree mostly with @Starcounter-Jack. To clear up a few tangents in this conversation:

  1. Polices are unrelated and not a good solution for this problem. The fact that once you have a policy you need to whitelist your entire dep graph is untenable. The OP gist shows how this could inter-op with our current file based resolution enabling only mapping partially, a huge ergonomic win IMO.
  2. Loaders are great for prototyping this, but in the long run direct support is the only reliable way. It would enable all package managers to have a reliable target to build against and will be important if we want this to see real usage.

Something I don't see as a tangent, but a serious and primary concern, spec progress and browser intent. As I have said above, I think we need to continue to align with other JS runtimes, but being a follower (as @Starcounter-Jack points out) means we don't help shape solution to our needs. And in this case, we have very clear needs which we want accounted for.

In another thread on the import maps repo, Dominic volunteered @MylesBorins for the job of representing this between Node and the spec. This was the second time I have seen something from that spec overlap with the modules group. I obviously cannot and will not speak for Myles, but I think if we can have alignment on what we would want import maps to do, we should make a clear and concerted effort to participate in the spec process as well. Anything less will end in more pain than necessary. I have no spec writing experience, but am happy to participate in building this alignment.