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

Incorrect substitution with multiple * wildcards #9

Closed
evanw opened this issue Mar 8, 2021 · 2 comments
Closed

Incorrect substitution with multiple * wildcards #9

evanw opened this issue Mar 8, 2021 · 2 comments

Comments

@evanw
Copy link

evanw commented Mar 8, 2021

I looked at this library briefly when I implemented this feature for esbuild and discovered some bugs. One of them is that it looks like this library doesn't handle * wildcards correctly when there are multiple wildcards:

const re = require('resolve.exports')
const pkg = {
  exports: {
    "./dir*": "./*sub/dir*/file.js"
  }
}
console.log(re.resolve(pkg, 'dirtest'))

Expected output:

./testsub/dirtest/file.js

Observed output:

./testsub/dir*/file.js

The relevant part from the spec is this one:

  1. If pattern is true, then
    1. Return the URL resolution of resolvedTarget with every instance of "*" replaced with subpath.

Other bugs I found included #7 and missing validation checks. I wasn't sure whether to also file a bug about the missing validation checks since you could argue that it's potentially not a concern of this library, although if many tools are adopting this package then it would probably make sense to put the validation checks in one place instead of replicating them in the various projects.

@lukeed
Copy link
Owner

lukeed commented Mar 8, 2021

Oh right, missed this. Thank you!

Validation checks of the target path(s) are implemented as part of #6 – I'm still waiting on general consensus as to whether or not others feel this library should be responsible for them.

Hope you found this lib somewhat helpful for your implementation :)

@huntie
Copy link
Contributor

huntie commented Dec 20, 2022

Hmm, I'm not sure this is the intended resolution behaviour in the spec.

Here you've quoted a step from the PACKAGE_TARGET_RESOLVE function in the resolution algorithm.

i. Return the URL resolution of resolvedTarget with every instance of "*" replaced with subpath.

However, this is incongruent with:

  1. The definition of PACKAGE_IMPORTS_EXPORTS_RESOLVE (relevant to the package.json exports field) which explicitly notes the single wildcard handling (pattern trailer).

    image

    This does depend on PACKAGE_TARGET_RESOLVE but again explicitly passes null to the patterns argument (meaning substitution wouldn't be implemented at that level).

    image

  2. The examples in the Subpath patterns section of the Packages spec, and the note on the implementing PR:

  1. Multiple * can't be matched in patterns, so given "exports": { ".//.js": "./.js" } this will no longer match import './/x.js' and instead throw an unexported error. — module: support pattern trailers nodejs/node#39635 (comment)

Disclaimer: End-of-day pseudocode reading and I could have missed something above — please correct me if so!

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

No branches or pull requests

3 participants