Repository navigation
Ambiguous steps in PACKAGE_IMPORTS_EXPORTS_RESOLVE reference documentation #53206
Description
Affected URL(s)
https://nodejs.org/api/esm.html#resolution-algorithm-specification
Description of the problem
I was looking at the ESM resolution algorithm documentation, and stumbled upon this:
PACKAGE_IMPORTS_EXPORTS_RESOLVE:
- [...]
- Let expansionKeys be the list of keys of matchObj containing only a single "*", sorted by the sorting function PATTERN_KEY_COMPARE which orders in descending order of specificity.
- [...]
(2) could be interpreted in several ways, including:
- List of keys of matchObj, sorted, filtered by keys containing only a single "*".
- List of keys of matchObj, filtered by keys containing only a single "*", sorted. I think the end result is the same in this case, though.
- List of keys of matchObj, where the value contains only a single "*", sorted by keys.
Assuming (1) or (2) here is correct, the implementation of PATTERN_KEY_COMPARE seems odd:
[...]
3. Let baseLengthA be the index of "" in keyA plus one, if keyA contains "", or the length of keyA otherwise.
4. Let baseLengthB be the index of "" in keyB plus one, if keyB contains "", or the length of keyB otherwise.
[...]
7. If keyA does not contain "", return 1.
8. If keyB does not contain "", return -1.
[...]
From my understanding, neither keyA nor keyB can not contain a "*" at this point since it's filtered out. I looked at implementations of the module resolution, like Webpack's enhanced-resolve, and it seems like previously these imports or exports could end with a / instead of containing a *:
However, this does not seem to work in the current Node.js version (tested on v20.12.2 at least), and is not described in the reference of PACKAGE_IMPORTS_EXPORTS_RESOLVE. Are the docs outdated? Is there some ambiguity here? Or am I simply misunderstanding the docs?
Activity
avivkeller commentedon May 30, 2024
@nodejs/loaders
(BTW 1 & 2 return the same result)
Mrtenz commentedon May 30, 2024
I figured that as well after writing it. 😅 Still, there is some ambiguity here, since it could also be interpreted as (3).
aduh95 commentedon May 30, 2024
I personally don’t find it confusing (we’re talking about the key, that would make little sense to involve the value at this point), and even if you misinterpret it, it wouldn’t be too bad because the value would also typically contain a single
*. As always, PRs welcome to clarify the docs.Mrtenz commentedon May 30, 2024
Fair enough. But if it's the case that the key always contains a single "*", the reference of
PATTERN_KEY_COMPAREseems to have unnecessary steps. Happy to open a PR to update this.aduh95 commentedon May 30, 2024
Let me rephrase that: value associated with keys that contains a single
*char will typically also contain a single*char. Keys do not always contain a single*char.Example of a valid
"exports"map:{ "exports": { "./someKey/noStar": "./someFile.mjs", "./somePatternKey/*": "./someDir/*.mjs" } }Mrtenz commentedon May 30, 2024
I understand that some keys don't have a "*", but the reference for
PACKAGE_IMPORTS_EXPORTS_RESOLVEmentions:So I assume an implementation for this in JS could be:
After which
expansionKeysis sorted byPATTERN_KEY_COMPARE. YetPATTERN_KEY_COMPAREincludes cases for when a key does not contain "*", which is never the case, right? It's never called for keys that don't contain a "*".aduh95 commentedon May 30, 2024
Ah I see, sorry for the misunderstanding! I think that's a relic from a time where there was another another way to define subpath patterns which was removed in #40121, and I guess
PATTERN_KEY_COMPAREwas never updated, good catch. If you want to PR a fix, that would be much appreciated.Mrtenz commentedon May 30, 2024
Opened a PR here: #53215.
doc: remove cases for keys not containing "*" in PATTERN_KEY_COMPARE
doc: remove cases for keys not containing "*" in PATTERN_KEY_COMPARE
doc: remove cases for keys not containing "*" in PATTERN_KEY_COMPARE