Skip to content
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

fix: support pattern trailers #361

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

jedwards1211
Copy link

@jedwards1211 jedwards1211 commented Sep 9, 2023

fixes #266

Supports trailing characters after the * in wildcard exports patterns, e.g.

    "./*.js": {
      "module": "./*.js",
      "default": {
        "node": "./*b.js"
      }
    },

I copied the logic from Node's resolve implementation.

Copy link

@orca-security-us orca-security-us bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Orca Security Scan Summary

Status Check Issues by priority
Passed Passed Secrets high 0   medium 0   low 0   info 0 View in Orca

Copy link
Contributor

@guybedford guybedford left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for posting this, it is definitely a missing feature and the last missing feature before this resolver implementation can be considered complete (given this resolve feature landed in Node.js later itself).

I left a couple of comments. That said I don't think the implementation is complete and likely needs a closer look at the spec. It also needs tests to land.

If you need implementation inspiration, perhaps take a look at rollup/plugins#1549 which was very nicely written.

(a, b) => b.length - a.length
)) {
if (!match.endsWith("/")) continue;
for (const match of Object.keys(matchObj).sort((a, b) => b.length - a.length)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sort should be updated to use the order defined by PATTERN_KEY_COMPARE in https://nodejs.org/dist/latest-v20.x/docs/api/esm.html#resolution-and-loading-algorithm.

Comment on lines 42 to 43
if (job.ts && path.startsWith(job.base) && path.slice(job.base.length).indexOf(sep + 'node_modules' + sep) === -1 && await job.isFile(path + '.ts')) return path + '.ts';
if (job.ts && path.startsWith(job.base) && path.slice(job.base.length).indexOf(sep + 'node_modules' + sep) === -1 && await job.isFile(path + '.tsx')) return path + '.tsx';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps abstract this into an extensions configuration rather, so it can be configured and not necessarily the default (sincethis will match ./file.ts before file.js).

@jedwards1211
Copy link
Author

Okay I'll go over this again soon.

Side note: it seems like a shame to have all these separate implementations of the node module resolution algorithm. Is there really no package that's flexible enough to be used in all of these cases? I've used the resolve package some before, but I haven't investigated if it's lacking anything or why we wouldn't want to depend on it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add exports / imports pattern trailers support
2 participants