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

Support for Yarn PnP #655

Closed
stof opened this issue Oct 18, 2019 · 16 comments
Closed

Support for Yarn PnP #655

stof opened this issue Oct 18, 2019 · 16 comments

Comments

@stof
Copy link
Member

stof commented Oct 18, 2019

Yarn has a new installation mode called PnP, meant at deduplicating packages as much as possible on disk. The culprit of this mode is that each package can only require packages that it explicitly defines as dependencies (instead of being able to require any hoisted dependencies).

webpack-encore currently relies on being able to require extra packages in a bunch of places to handle optional dependencies. This won't work as is in PnP mode. The solution is to define optional peer dependencies for them (peer dependencies because we expect them to be required by a higher-level package and be available to us, and optional to avoid displaying a warning when they are not installed).
Note that npm currently has a (server-side) bug which ignores the peerDependenciesMeta field when using the npm CLI and so peer dependencies are not treated as optional there for the initial install and still display a warning (a fix is in progress). Yarn is not impacted by the bug.

Note that this will also improve things for non-PnP users, because hoisting could potentially break things when the package manager is not aware of peer dependencies because they are not declared. See the example in npm/cli#224

And a nice side-effect is that yarn/npm will warn when installing an incompatible version.

@stof
Copy link
Member Author

stof commented Jan 16, 2020

defining proper peer dependencies might also make encore compatible with pnpm (that I just discovered)

@weaverryan
Copy link
Member

The solution is to define optional peer dependencies

Do "optional peer dependencies" exist? Is that a thing you can actually define?

@stof
Copy link
Member Author

stof commented Feb 15, 2021

yes it is

@stof
Copy link
Member Author

stof commented Feb 15, 2021

see the peerDependenciesMeta field to mark them as optional

@shmolf
Copy link
Contributor

shmolf commented Jun 6, 2021

link to peerDependenciesMeta, for ease.

@floviolleau
Copy link

Hi,

Thanks for your amazing project!

Do we have any news to make it work with Yarn berry?

Thanks

@stof
Copy link
Member Author

stof commented Nov 17, 2021

Nobody has worked yet on adding those optional peer dependencies. So no, there is no news. But contributions are welcome to help.

@bazo
Copy link

bazo commented Mar 23, 2022

what exactly needs to be done? i would really like to use encore with yarn pnp
can do the work required

@stof
Copy link
Member Author

stof commented Mar 23, 2022

all the optional peer dependencies need to be defined

@bazo
Copy link

bazo commented Apr 8, 2022

i added tapable and webpack as peer deps in this commit
bazo@3e6c47b

but when i install the package, encore still produces error about missing tapable

so it seems this is not enough. any hints? thanks

@shmolf
Copy link
Contributor

shmolf commented Apr 10, 2022

@bazo Can we see the error output?

Also, can we try not making the dependency optional?

  "peerDependenciesMeta": {
    "tapable": {
-     "optional": true
+     "optional": false
    },
    "webpack": {
-     "optional": true
+     "optional": false
    }
  },

@bazo
Copy link

bazo commented Apr 11, 2022

@shmolf

this is the whole output

Running webpack ...

[webpack-cli] Failed to load 'project/webpack.config.js' config
[webpack-cli] Error: @symfony/webpack-encore tried to access tapable, but it isn't declared in its dependencies; this makes the require call ambiguous and unsound.

Required package: tapable
Required by: @symfony/webpack-encore@npm:1.8.2 (via project/.yarn/cache/@symfony-webpack-encore-npm-1.8.2-4b7aed6630-7dcd1dc7a3.zip/node_modules/@symfony/webpack-encore/lib/webpack-manifest-plugin/)

Require stack:
- project/.yarn/cache/@symfony-webpack-encore-npm-1.8.2-4b7aed6630-7dcd1dc7a3.zip/node_modules/@symfony/webpack-encore/lib/webpack-manifest-plugin/hooks.js
- project/.yarn/cache/@symfony-webpack-encore-npm-1.8.2-4b7aed6630-7dcd1dc7a3.zip/node_modules/@symfony/webpack-encore/lib/webpack-manifest-plugin/index.js
- project/.yarn/cache/@symfony-webpack-encore-npm-1.8.2-4b7aed6630-7dcd1dc7a3.zip/node_modules/@symfony/webpack-encore/lib/plugins/manifest.js
- project/.yarn/cache/@symfony-webpack-encore-npm-1.8.2-4b7aed6630-7dcd1dc7a3.zip/node_modules/@symfony/webpack-encore/lib/config-generator.js
- project/.yarn/cache/@symfony-webpack-encore-npm-1.8.2-4b7aed6630-7dcd1dc7a3.zip/node_modules/@symfony/webpack-encore/index.js
- project/webpack.config.js
- project/.yarn/__virtual__/webpack-cli-virtual-bd01c23246/0/cache/webpack-cli-npm-4.9.2-5e7d77ef6f-ffb4c5d53a.zip/node_modules/webpack-cli/lib/webpack-cli.js
- project/.yarn/__virtual__/webpack-cli-virtual-bd01c23246/0/cache/webpack-cli-npm-4.9.2-5e7d77ef6f-ffb4c5d53a.zip/node_modules/webpack-cli/lib/bootstrap.js
- project/.yarn/__virtual__/webpack-cli-virtual-bd01c23246/0/cache/webpack-cli-npm-4.9.2-5e7d77ef6f-ffb4c5d53a.zip/node_modules/webpack-cli/bin/cli.js
- project/.yarn/__virtual__/webpack-virtual-a6e600fb3d/0/cache/webpack-npm-5.72.0-efdd74e984-8365f1466d.zip/node_modules/webpack/bin/webpack.js
- project/.yarn/cache/@symfony-webpack-encore-npm-1.8.2-4b7aed6630-7dcd1dc7a3.zip/node_modules/@symfony/webpack-encore/bin/encore.js
    at Function.require$$0.Module._resolveFilename (project/.pnp.cjs:21718:13)
    at Function.require$$0.Module._load (project/.pnp.cjs:21572:42)
    at Module.require (node:internal/modules/cjs/loader:999:19)
    at require (node:internal/modules/cjs/helpers:102:18)
    at Object.<anonymous> (project/.yarn/cache/@symfony-webpack-encore-npm-1.8.2-4b7aed6630-7dcd1dc7a3.zip/node_modules/@symfony/webpack-encore/lib/webpack-manifest-plugin/hooks.js:4:31)
    at Module._compile (node:internal/modules/cjs/loader:1097:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1149:10)
    at Object.require$$0.Module._extensions..js (project/.pnp.cjs:21762:33)
    at Module.load (node:internal/modules/cjs/loader:975:32)
    at Function.require$$0.Module._load (project/.pnp.cjs:21602:14)

why do you think making the deps not optional would help?

@shmolf
Copy link
Contributor

shmolf commented Apr 11, 2022

why do you think making the deps not optional would help?

It seems that Encore/Webpack, during runtime, doesn't check whether tapable is "installed". It assumes it's existence, and I would consider that a requirement, not an optional one.

Reading up on the Yarn RFC, it looks like optional: true implies the parent application, or a sibling library, will be responsible for installing tapable. But as we know, that's not happening.

@shmolf
Copy link
Contributor

shmolf commented Apr 14, 2022

@bazo Any luck?

@Kocal
Copy link
Member

Kocal commented Aug 2, 2022

I've opened a PR which add support for Yarn PnP and PNPM: #1142

weaverryan added a commit that referenced this issue Aug 30, 2022
This PR was squashed before being merged into the main branch.

Discussion
----------

Add support for pnpm and Yarn PnP

For Yarn PnP and pnpm support, Encore needs to configure `peerDependenciesMeta` as explained in #655.

I've configured **every** packages listed in `lib/features.js` as **optional** peerDependencies, and `webpack` as a **required** peer dependency.

I'm not sure of how we can write tests for that, any idea `@weaverryan`? Maybe an E2E test with a real app/dedicated CI workflow?
**EDIT:** implemented E2E tests for Yarn PnP and pnpm.

Thanks!

Commits
-------

dbf3db4 Add support for pnpm and Yarn PnP
@Kocal
Copy link
Member

Kocal commented Sep 18, 2022

Can be closed since Encore v4.

@stof stof closed this as completed Sep 18, 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

No branches or pull requests

6 participants