Remove most pnpm hoisting, fix phantom deps #24096
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We've long had pnpm's public hoisting disabled, but it also privately hoists everything into `node_modules/.pnpm/node_modules/` for compatibility with packages that have undeclared dependencies. Let's disable most of that private hoisting and see what turns up. Changes of note: * We still hoist a bunch of eslint plugins (and one prettier plugin) as untangling that mess seems like enough work to put off for later. * Only a few upstream packages have bugs that need working around: * `@mdx-js/loader`: mdx-js/mdx#2019 * `@automattic/components`: Missing dep on `@wordpress/base-styles`. And the next version will probably be unusable for us due to added `i18n-calypso`. * `@automattic/popup-monitor`: Missing dep on `events`. * markdown-it`: Missing dep on `punycode`. markdown-it/markdown-it#230 * `@samverschueren/stream-to-observable`: Outdated dep on `any-observable`. SamVerschueren/stream-to-observable#9 Hacking around that should also fix p1649254510834369-slack-CBG1CP4EN. * `fetch-mock`'s peer dep on `node-fetch` is optional, but only because they also allow running in-browser. It's required for node. * Added `webpack-cli` alongside `webpack` everywhere to make sure p1650571211251179-slack-CBG1CP4EN is fixed. Dropped Webpack entirely from Boost instead though, they don't use it. * Had our webpack-config package point to its own copy of `@babel/runtime` instead of making that a peer dep. * Jetpack's extensions tests use a ton of `@wordpress/` packages that weren't being directly depended on.
|
Caution: This PR has changes that must be merged to WordPress.com |
github-actions
bot
added
[JS Package] API
[JS Package] Connection
[JS Package] Components
[JS Package] I18n Loader Webpack Plugin
[JS Package] I18n Check Webpack Plugin
[JS Package] Remove Asset Webpack Plugin
[JS Package] Storybook
[JS Package] Webpack Config
[Package] Ad
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available. Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped. Jetpack plugin:
Boost plugin:
|
anomiex
added
[Status] Needs Review
jeherve
approved these changes
Apr 27, 2022
jeherve
added
[Status] Ready to Merge
|
r244512-wpcom |
|
Great news! One last step: head over to your WordPress.com diff, D79551-code, and deploy it. Thank you! |
github-actions
bot
removed
the
[Status] Ready to Merge
samiff
pushed a commit
that referenced
this pull request
Apr 27, 2022
We've long had pnpm's public hoisting disabled, but it also privately hoists everything into `node_modules/.pnpm/node_modules/` for compatibility with packages that have undeclared dependencies. Let's disable most of that private hoisting and see what turns up. Changes of note: * We still hoist a bunch of eslint plugins (and one prettier plugin) as untangling that mess seems like enough work to put off for later. * Only a few upstream packages have bugs that need working around: * `@mdx-js/loader`: mdx-js/mdx#2019 * `@automattic/components`: Missing dep on `@wordpress/base-styles`. And the next version will probably be unusable for us due to added `i18n-calypso`. * `@automattic/popup-monitor`: Missing dep on `events`. * markdown-it`: Missing dep on `punycode`. markdown-it/markdown-it#230 * `@samverschueren/stream-to-observable`: Outdated dep on `any-observable`. SamVerschueren/stream-to-observable#9 Hacking around that should also fix p1649254510834369-slack-CBG1CP4EN. * `git-node-fs`: Missing peer dep on `js-git`. creationix/git-node-fs#8 * `fetch-mock`'s peer dep on `node-fetch` is optional, but only because they also allow running in-browser. It's required for node. * Added `webpack-cli` alongside `webpack` everywhere to make sure p1650571211251179-slack-CBG1CP4EN is fixed. Dropped Webpack entirely from Boost instead though, they don't use it. * Had our webpack-config package point to its own copy of `@babel/runtime` instead of making that a peer dep. * Jetpack's extensions tests use a ton of `@wordpress/` packages that weren't being directly depended on.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes proposed in this Pull Request:
We've long had pnpm's public hoisting disabled, but it also privately
hoists everything into
node_modules/.pnpm/node_modules/forcompatibility with packages that have undeclared dependencies. Let's
disable most of that private hoisting and see what turns up.
Changes of note:
untangling that mess seems like enough work to put off for later.
@mdx-js/loader:@mdx-js/loaderv1 does not work with yarn pnp or pnpm mdx-js/mdx#2019@automattic/components: Missing dep on@wordpress/base-styles.And the next version will probably be unusable for us due to added
i18n-calypso.@automattic/popup-monitor: Missing dep onevents.: Missing dep onpunycode`.Library requires punycode but not in package.json markdown-it/markdown-it#230
@samverschueren/stream-to-observable: Outdated dep onany-observable.update any-observable SamVerschueren/stream-to-observable#9
Hacking around that should also fix p1649254510834369-slack-CBG1CP4EN.
git-node-fs: Missing peer dep onjs-git.fix: add missing optional peer dependency
js-gitcreationix/git-node-fs#8fetch-mock's peer dep onnode-fetchis optional, but only becausethey also allow running in-browser. It's required for node.
webpack-clialongsidewebpackeverywhere to make surep1650571211251179-slack-CBG1CP4EN is fixed. Dropped Webpack entirely
from Boost instead though, they don't use it.
@babel/runtimeinstead of making that a peer dep.@wordpress/packages thatweren't being directly depended on.
Jetpack product discussion
None
Does this pull request change what data or activity we track or use?
No
Testing instructions: