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 glob resolver package issue with deep entry points #8169

Conversation

marcins
Copy link
Contributor

@marcins marcins commented May 30, 2022

↪️ Pull Request

This is a follow-up from #8097 - when trying to implement the change in my repository I realised there was a step missed, if the package entry point resolves to a file not in the root, then the paths for the glob import will need to be relative to the entry point, where they should be able to be relative to the package root.

This change adds an additional step to perform a findPackage on the resulting filePath in order to resolve to the root of the package.

💻 Examples

Before if you had a package @scope/pkg with an entry point of ./dist/index.js then the glob resolver would resolve the root to @scope/pkg/dist/, so if you used a path like @scope/pkg/src/foo*.js it would actually resolve to @scope/pkg/dist/src/foo*.js which is incorrect. With this change the path resolves as expected.

🚨 Test instructions

Failing integration test with a deep entry point was added, change was made and confirmed all integration tests still pass.

✔️ PR Todo

  • Added/updated unit integration tests for this change
  • Filled out test instructions (In case there aren't any unit tests)
  • Included links to related issues/PRs

@marcins marcins changed the title Mszczepanski/glob resolver fix pkg root issue Fix glob resolver package issue with deep entry points May 30, 2022

specifier = path.resolve(path.dirname(result.filePath), rest);
const ctx = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wasn't sure what to do about this - findPackage requires a context, so I provided one with the invalidation info from the step above, however when we return the resolution at the end of this method around line 178, there's only invalidateOnFileCreate: [{glob: normalized}], included. Should we collect all these additional invalidations and add them there as well?

loc: dependency.loc,
};

const resolvedPackage = await resolver.findPackage(filePath, ctx);
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure this is quite right because there could be another package.json inside a sub-directory of the module. We can use the resolver.resolveModule method for this I think.

@devongovett devongovett force-pushed the mszczepanski/glob-resolver-fix-pkg-root-issue branch from 06b1756 to 4d86d64 Compare June 13, 2022 00:29
@devongovett devongovett merged commit b837522 into parcel-bundler:v2 Jun 13, 2022
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.

None yet

2 participants