PATTERN_KEY_COMPARE algorithm should be updated

[unional]

Version

19.3.1

Platform

all

Subsystem

No response

What steps will reproduce the bug?

https://nodejs.org/api/esm.html#resolver-algorithm-specification

The assertions: Assert: keyA ends with "/" or contains only a single "*".

is no longer correct as ESM supports file extensions, e.g.:

• #a/b.js
• ./foo/*.js

I have released pattern-key-compare for resolve.imports.

How often does it reproduce? Is there a required condition?

N/A

What is the expected behavior?

The spec should be adjusted.

Also, should expect only single * instead of allowing multiple *. But that's a breaking change.

What do you see instead?

N/A

Additional information

No response

[aduh95]

I'm not sure I understand what you mean, how is it not correct?

• #a/b.js is not a pattern, so it's expected that it breaks the assertion I think.
• ./foo/*.js contains only a single *, so the assertion passes.

Do you have example of valid patterns that break the assertion?

[unional]

AFAIK this is used to sort the keys in the exports and imports field.
i.e. in https://nodejs.org/api/esm.html#resolver-algorithm-specification

PACKAGE_IMPORTS_EXPORTS_RESOLVE()
2. 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.

And you can see many of the keys in https://nodejs.org/api/packages.html#package-entry-points does not satisfy this assertion. e.g.:

{
  "name": "my-package",
  "exports": {
    ".": "./lib/index.js",
    "./lib": "./lib/index.js",
    "./lib/*": "./lib/*.js",
    "./lib/*.js": "./lib/*.js",
    "./feature": "./feature/index.js",
    "./feature/*": "./feature/*.js",
    "./feature/*.js": "./feature/*.js",
    "./package.json": "./package.json"
  }
}

// ./node_modules/es-module-package/package.json
{
  "exports": {
    "./features/*.js": "./src/features/*.js"
  },
  "imports": {
    "#internal/*.js": "./src/internal/*.js"
  }
}

[unional]

Also, maybe the wording is a bit ambiguous:

Assert: keyA ends with "/" or contains only a single "*".

Does it mean:

• keyA (ends with "/") or (contains only a single "*"), or
• keyA ends with ("/" or contains only a single "*")

I can understand it is probably the prior, but you can see that popular package resolve.exports got the wrong idea.

[unional]

Let expansionKeys be the list of keys of matchObj containing only a single "*"

Reading this again, seems like the first assertion is not even needed, as the PATTERN_KEY_COMPARE is only used there in the whole algorithm. Maybe historical?

[aduh95]

Does it mean:

• keyA (ends with "/") or (contains only a single "*"), or
• keyA ends with ("/" or contains only a single "*")

I'm not a native English speaker, so maybe there are nuances I don't get, but I don't think "ends with contains only a single *" is valid grammar and I wouldn't know how to interpret it, so to me only the former proposal makes sense and the assertion is not ambiguous.

Let expansionKeys be the list of keys of matchObj containing only a single "*"

Reading this again, seems like the first assertion is not even needed, and the PATTERN_KEY_COMPARE is only used there in the whole algorithm. Maybe historical?

This was added in #39635 by @guybedford, maybe he can help here?

[guybedford]

@unional I believe you're correct - the ends with "/" is legacy and can be removed at this point from the assertion.

[unional]

Sounds good. thx @guybedford. I can open a PR and also update the respective code.

[github-actions]

This issue has been marked as stale due to 210 days of inactivity.
It will be automatically closed in 30 days if no further activity occurs. If this is still relevant, please leave a comment or update it to keep it open.

[github-actions]

This issue has been automatically closed after 30 days of inactivity following its stale status (no activity for a total of 120 days).
If this is still relevant, feel free to reopen it or leave a comment with additional details so we can continue the discussion.