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

use fs.realpath to resolve module locations #2946

Closed
wants to merge 1 commit into from

Conversation

torbensky
Copy link

↪️ Pull Request

This updates the way node modules are resolved by the Parcel resolver such that the "real path" is used. This way it matches the node resolution algorithm and makes Parcel compatible with other tools in the node ecosystem that rely on this behavior (like pnpm).

See issue #1125 which this PR would address.

💻 Examples

A pnpm workspace has this issue with transitive dependencies. For example, if a module in the workspace uses React, then Parcel will not find React's transitive dependencies (usually failing to find object-assign).

pnpm sets up a folder structure like this:

monorepo
├── web-ui <-- parcel run in this module
│   ├── package.json
│   ├── node_modules
│           ├── react <-- symlinked to the .registry.npmjs.org/react
├── node_modules
│       ├── .registry.npmjs.org
│               ├── react
│               ├── object-assign

Because Parcel ignores the real path, it tries to find object-assign like this:

  • /monorepo/web-ui/node_modules/react/node_modules
  • /monorepo/web-ui/node_modules/
  • /monorepo/node_modules <-- not found here because it's in subfolder .registry.npmjs.org
  • etc...

🚨 Test instructions

TODO: I may be able to contribute some tests, looking for guidance here

✔️ PR Todo

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

@hgossler
Copy link

hgossler commented Sep 9, 2019

Any updates on this?

hgossler added a commit to hgossler/parcel that referenced this pull request Sep 10, 2019
Addition of bin field allows to use this repo as dependency directly
from github.

Source for fix: parcel-bundler#2946
hgossler added a commit to hgossler/parcel that referenced this pull request Sep 10, 2019
Allows to use this repo as dependency directly
from github.

Source for fix: parcel-bundler#2946
@DeMoorJasper
Copy link
Member

Closing due to inactivity

@Mstrodl
Copy link

Mstrodl commented Apr 12, 2020

@DeMoorJasper This is still an issue, I really think this should be reopened. There's not much more to say on this, it still works on the latest version...

@scottned
Copy link

scottned commented Jun 3, 2020

Please reopen and merge this. Parcel is unusable for us without this patch.

@DeMoorJasper
Copy link
Member

DeMoorJasper commented Jun 3, 2020

@scottned Parcel 1 is in maintenance mode so this probably wouldn't get released anyway if merged at this point.

Feel free to port this to Parcel 2 if it's still an issue there. A beta for Parcel 2 will come out soon

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.

5 participants