facebook / relay

Relay is a JavaScript framework for building data-driven React applications.
https://relay.dev
MIT License
18.36k stars 1.82k forks source link

Support Yarn PnP #2698

Open steida opened 5 years ago

steida commented 5 years ago

I am playing with Yarn PnP and it seems, that relay-compiler needs to update its dependencies. Am I correct?

Error: Package "relay-compiler@3.0.0" (via "/Users/steida/Library/Caches/Yarn/v4/npm-relay-compiler-3.0.0-689015f2aa638296c82c5761e5b4cdb09aed7139/node_modules/relay
-compiler/bin/relay-compiler") is trying to require the package "core-js" (via "core-js/es6") without it being listed in its dependencies (graphql, @babel/generator, @b
abel/parser, @babel/polyfill, @babel/runtime, @babel/traverse, @babel/types, babel-preset-fbjs, chalk, fast-glob, fb-watchman, fbjs, immutable, nullthrows, relay-runtim
e, signedsource, yargs, relay-compiler)
 |     at makeError (/Users/steida/dev/este/.pnp.js:55:17)
 |     at Object.resolveToUnqualified (/Users/steida/dev/este/.pnp.js:16133:17)
 |     at Object.resolveRequest (/Users/steida/dev/este/.pnp.js:16204:31)
 |     at Function.Module._resolveFilename (/Users/steida/dev/este/.pnp.js:16386:30)
 |     at Function.Module._load (/Users/steida/dev/este/.pnp.js:16302:31)
 |     at Module.require (module.js:596:17)
 |     at require (packages/relay/internal/module.js:11:18)
 |     at Object.<anonymous> (/Users/steida/Library/Caches/Yarn/v4/npm-relay-compiler-3.0.0-689015f2aa638296c82c5761e5b4cdb09aed7139/node_modules/relay-compiler/bin/rel
ay-compiler:14659:18)
 |     at __webpack_require__ (/Users/steida/Library/Caches/Yarn/v4/npm-relay-compiler-3.0.0-689015f2aa638296c82c5761e5b4cdb09aed7139/node_modules/relay-compiler/bin/re
lay-compiler:30:30)
 |     at Object.<anonymous> (/Users/steida/Library/Caches/Yarn/v4/npm-relay-compiler-3.0.0-689015f2aa638296c82c5761e5b4cdb09aed7139/node_modules/relay-compiler/bin/rel
ay-compiler:9651:1)
robrichard commented 5 years ago

I think the issue is here:

https://github.com/facebook/relay/blob/master/gulpfile.js#L119

Relay compiler is bundled by webpack into bin/relay-compiler. I think the intention of the externals regex was to exclude any third party modules from the bundle, but it does not match @babel/polyfill.

So that package is inlined into relay-compiler but it's imports from core-js are not. Now to yarn pnp it looks like relay-compiler is importing core-js directly without specifying it as a dependency.

koistya commented 4 years ago

Fixed by adding missing dependencies to packageExtensions:

packageExtensions:
  babel-plugin-relay@*:
    dependencies:
      "@babel/runtime": ^7.12.5
  relay-compiler@*:
    dependencies:
      relay-compiler-language-typescript: ^13.0.2
      relay-config: ^10.1.3
josephsavona commented 4 years ago

We're open to PRs to extend Relay to support PnP, but this isn't something we will likely prioritize ourselves until PnP is widely adopted.

arcanis commented 4 years ago

The problem is that the language server is loaded through a direct require call, so it relies on the specific hoisting layout (if a sibling dependency simply happens to depend on the language plugin, relay-compiler may not find the root one anymore).

Since the plugin is loaded from the cwd a few lines above, I believe an improvement would be to do the same for the plugin resolution, using the paths option from require.resolve:

const req = eval('require');
languagePlugin = req(req.resolve(requirePath, {
  paths: [process.cwd()],
}));

In the meantime, and as a workaround, you can use packageExtensions to force-declare a dependency on relay-compiler-language-typescript (or any other language) to relay-compiler.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

xyy94813 commented 3 years ago

How prevalent is PNP in 2021? Is the feature being considered for continued promotion?

I am evaluating whether to use PnP in my Relay project.

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

xyy94813 commented 2 years ago

Can I use pnpm as an alternative?