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(remix-dev): fix Yarn PnP resolve issue #3579

Merged
merged 1 commit into from
Jun 28, 2022

Conversation

alisd23
Copy link
Contributor

@alisd23 alisd23 commented Jun 26, 2022

Closes: (no issue yet - can create one if you like)

  • Docs
  • Tests

Testing Strategy:
There isn't currently an easy way for us to test yarn pnp / Remix compatibility, so this is just a local manual test.

Before this change (current dev branch, when running remix build, multiple errors (as below) appear, I guess because yarn cannot resolve the paths generated by the NodeModulesPolyfills plugin, unless the esbuild-plugin-pnp plugin runs first. I'm guessing the pnp plugin needs to set up some path transformation stuff before the polyfill plugin, but the docs for both are super light so I'm not 100% sure why it fixes the errors, but it does.

Thankfully when not using yarn pnp, esbuild-plugin-pnp is a no-op, so no risk to non-yarn users either way.

Errors before change

Build failed with 9 errors:
node-modules-polyfills:http:30:26: ERROR: [plugin: @yarnpkg/esbuild-plugin-pnp] Qualified path resolution failed: we looked for the following paths, but none could be accessed.

node-modules-polyfills:https:30:26: ERROR: [plugin: @yarnpkg/esbuild-plugin-pnp] Qualified path resolution failed: we looked for the following paths, but none could be accessed.

node-modules-polyfills:stream:4:21: ERROR: [plugin: @yarnpkg/esbuild-plugin-pnp] Qualified path resolution failed: we looked for the following paths, but none could be accessed.

node-modules-polyfills:stream:5:23: ERROR: [plugin: @yarnpkg/esbuild-plugin-pnp] Qualified path resolution failed: we looked for the following paths, but none could be accessed.

node-modules-polyfills:stream:6:23: ERROR: [plugin: @yarnpkg/esbuild-plugin-pnp] Qualified path resolution failed: we looked for the following paths, but none could be accessed.

@alisd23 alisd23 changed the base branch from main to dev June 26, 2022 14:24
@alisd23 alisd23 force-pushed the fix/yarn-pnp-node-modules-polyfill branch from a8ebae5 to 54b64a6 Compare June 26, 2022 14:24
@remix-cla-bot
Copy link
Contributor

remix-cla-bot bot commented Jun 26, 2022

Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳

@alisd23 alisd23 force-pushed the fix/yarn-pnp-node-modules-polyfill branch from 54b64a6 to 5642a64 Compare June 26, 2022 14:25
@alisd23 alisd23 changed the title Fix yarn pnp (node modules polyfill) Fix yarn pnp (node-modules-polyfills) Jun 27, 2022
@MichaelDeBoey MichaelDeBoey changed the title Fix yarn pnp (node-modules-polyfills) fix(remix-dev): fix Yarn PnP resolve issue Jun 27, 2022
@pcattori pcattori merged commit c68ae7b into remix-run:dev Jun 28, 2022
@alisd23 alisd23 deleted the fix/yarn-pnp-node-modules-polyfill branch July 1, 2022 22:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants