module: package "imports" field

[guybedford]

This is a follow-up to #33780 that implements the full package.json "imports" field, as specified in https://github.com/jkrems/proposal-pkg-exports#3-imports-field.

See the specification there or the docs updates in this PR for the full implementation details but the gist of it is:

• #specifier means a private internal "imports" resolution.
• "imports": { "#specifier": "./asdf.js" } permits local package mappings
• "imports": { "#specifier": "dependency/asdf.js" } permits external mappings.

The main justification for this is to enable the features of the "exports" field to fully cover what was provided by the "browser" field such that the "exports" and "imports" fields can entirely replace these use cases.

For example, if a package only wants to resolve an optional dependency when running in Node.js, but want to use a local polyfill otherwise you can define an "imports" field like:

{
  "imports": {
    "#dep": {
      "node": "dep-node-native",
      "default": "./local-polyfill.js"
    }
  },
  "optionalDependencies": {
    "dep-node-native": "1.2.3"
  }
}

Then any use of require('#dep') or import('#dep') within the package itself will resolve appropriately where the local polyfill is used when not in a Node.js environment allows the extension of conditional resolution provided by the exports field to apply within a package itself.

The sigil "#" is used to properly disambiguate these internal resolutions. In the CommonJS implementation, full backwards compatibility is provided for node_modules/#pkg packages that happen to start with #, even though these are likely very rare.

This PR fully shares the code that resolves both "exports" and "imports" so provides some significant refactorings including to error handling cases.

It is also the first case of full code sharing between the CJS and ESM resolvers thanks to @addaleax porting the ESM resolver to JS. This made this PR much easier than it would have been otherwise.

The ability for internal resolutions to map to external packages is a new unique feature not offered by the existing "exports" field. This is a benefit over the previous PR at #33780 and is enabled by the fact that circular resolutions are not possible for "imports" as they are for "exports".

There is certainly some added complexity here - but at the same time this closes the full loop on providing the same functionality for the "browser" field through this "imports" analog to the "exports" field, which is quite important for cross-environment use cases and does still come up quite a bit practically.

I have recently discussed the need for this PR in practical tooling scenarios with both @mafintosh and @sokra who as far as I could tell are in support of this work.

The tests and implementation are fairly comprehensive. We should also aim to land this while we are still within the window to backport to v12.

[jkrems]

It is also the first case of full code sharing between the CJS and ESM resolvers thanks to @addaleax porting the ESM resolver to JS. This made this PR much easier than it would have been otherwise.

This makes me really exciting to dig into the changes! 🎉

[jkrems]

I think this will help getting a consistent set of metadata for describing resolution for JavaScript (independent of where it runs). Using # ("private field marker") seems consistent with JavaScript terms and since this happens before conversion to URLs, it shouldn't conflict / should be web-compatible.

[jkrems]

OT: This algorithm is pretty much un-reviewable at this point (at least to me). I don't think it's in scope for this PR but maybe the answer is to move it to its own page and rethink how it's organized?

[guybedford]

I'm totally open to suggestions. My main concerns are keeping the API nature of the docs straightforward and having a nice link to this spec as it should be a reference. I do generally not like having it in a details element because it makes it difficult to link this spec without it being overlooked so I'd be all for a dedicated page actually.

[guybedford]

Forgot to cc @nodejs/modules-active-members here.

[guybedford]

Thanks @jkrems for the quick review here, just let me know if you have further feedback.

[bmeck]

we might want to add that fragments and queries are not preserved for non-directory paths.

[jkrems]

I assume they are neither preserved nor removed..? It may make sense to call out that for bare specifiers ?/#/etc. are treated like any other character if we don't do that already.

[bmeck]

looking at the markdown algorithm they would be removed it seems.

[jkrems]

I tried to find the part where it removes anything from the specifier and I can't find a reference to "?"/"#" or "query"/"fragment". According to my reading of the markdown algorithm (and memory of the spec), the following should work:

// ./package.json
{
  "name": "pkg",
  "imports": {
    "#/some?private#specifier": "dep/this?is?all#just%chars"
  }
}

// ./entry.mjs
import '#/some?private#specifier';

// ./node_modules/dep/package.json
{
  "name": "pkg",
  "exports": {
    "./this?is?all#just%chars": "./foo.mjs?some=query"
  }
}

The import in entry.mjs should load ./node_modules/dep/package.json/foo.mjs?some=query. It does honor the entire specifier, including the bits that happen to follow a question mark. So "not preserved" could be misunderstood I think. Similar behavior can also be observed in directory "exports", so this isn't specific to exact match:

{
  "exports": {
    "./this?may#contain/other/chars/": "./lib/"
  }
}

[bmeck]

the point is that

{
  "field (you used exports but this PR is imports)": {
    "#/x?y" : "./x"
  }

import '#/x?z';

would not carry ? or # parts of the URL from input to right hand side. It also needs those to match the left hand side if they are present. I am not willing to state that these left hand side strings are not URL based/like as that goes back to my problem with putting # in the exports field of making the switch of being an opaque value/string quite confusing and not matching any other specifier. I do think this is a difference that was fine in "exports" since it is a single location/workflow for the exception but as we grow the usage of these non relative non propagating specifiers we need to document the actual workflow. Even reading your comment above I am unclear on your meaning which is concerning on its own.

[guybedford]

@bmeck like import maps, the left hand side is always a direct string replacement, so the only difference remains the standard CJS v ESM resolvers handling resolution differently for those cases, but this applies equally to any absolute or relative paths - imports and exports targets are no different.

@jkrems note that I have disallowed #/ as a mapping prefix explicitly so your example wouldn't work and instead give an invalid specifier error. I thought it seemed sensible to lock down these base cases, but happy to reconsider.

[ljharb]

So to make sure i understand: “imports” would only be accessible via a package self-reference, and would not imply an “exports” entry?

Can “exports” use an “imports” LHS as a RHS value?

The # appears in the LHS of “imports” - is it required in the in-code specifier?

[sokra]

So to make sure i understand: “imports” would only be accessible via a package self-reference, and would not imply an “exports” entry?

I understand it this way:

No. imports is not related to self-reference. You use it this way: import "#foo" and not import "package#foo".

Can “exports” use an “imports” LHS as a RHS value?

No. exports allows only paths inside of the package.

The # appears in the LHS of “imports” - is it required in the in-code specifier?

Yes. See first comment.

[ljharb]

Can’t an existing folder in node_modules start with a #?

[guybedford]

@ljharb yes it is possible in theory. Like "exports", this feature is only enabled for CommonJS when the package has an "imports" field so that we get backwards compatibility.

[ljharb]

What would happen if foo/#bar is a valid path, and they add the imports field? Would they have to add a “#bar” import field value in order to make that backwards compatible?

[guybedford]

Relative path loading is entirely unaffected by this PR - the only case where require('#bar') might have load differently is when it is a valid node_modules lookup, which is only affected when the package author explicitly adds the imports field.

[ljharb]

Thanks for clarifying!

[guybedford]

Is there any further feedback on this PR? Is there anything we should be discussing in the meeting with regards to this?

@bmeck let me know if I've answered all your questions above.

[bmeck]

@guybedford the explanation seems fine but it still isn't in the docs.

[bmeck]

Can amend docs later

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/32246/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/32262/