Use npm workspaces for packages #65681
Conversation
t-hamano
added
the
[Type] Project Management
t-hamano
force-pushed
the
managing-packages
branch
from
ba88ca0 to
de0caa2
Compare
|
Size Change: 0 B Total Size: 1.77 MB ℹ️ View Unchanged
|
t-hamano
changed the title
t-hamano
force-pushed
the
managing-packages
branch
from
de0caa2 to
d99f87c
Compare
|
I'm trying to enable npm workspaces in the Gutenberg project so that dependencies are updated properly, but I'm running into two issues. If you have any approaches to resolve these, please let me know 🙇♂️
|
|
I’m curious if @kevin940726 run into the same issue when exploring namespaces in the past: @sirreal or @desrosj, do you have some hints to share? We definitely need to have a way to distinguish production and development packages. Maybe React Native packages need to be explicitly marked as dev packages. |
|
Thanks for working on this, I think it needs to happen 🙂 I need to investigate a bit more, but my first intuition is that maybe it's working correctly and the updates are as they should be 🤷 For example, installing a new dev dependency that isn't currently present is correctly represented as a dev dep. Try this: npm i -D oxlint -w @wordpress/a11y |
|
If we go in this direction, I wonder if the root package.json file should contain much less. An interesting question to consider is "what does the Gutenberg package.json represent?" It's entirely possible this just works differently than lerna and it is correct under the npm workspaces model. Here's a case:
Does the package-lock represent the root package, or is it more like a union of everything in the workspace? It may be the latter, in that case since these are dependencies of a package in the workspace, they should be dev dependencies. This is interesting: npm i -D @actions/coreThis changes the lockfile 🤦 it adds a license field in a few places where the package appears. I've never found npm to be very reliable and it continues today. Here's an interesting diff, I compared the following command on trunk and the branch: npm ls --package-lock-only --include=prod --parseablediff of prod packagesdiff --git before/trunk.txt after/branch.txt
index c8f325917e..f539055038 100644
--- before/trunk.txt
+++ after/branch.txt
@@ -69,6 +69,7 @@ node_modules/@wordpress/components
node_modules/@wordpress/compose
node_modules/@wordpress/core-commands
node_modules/@wordpress/core-data
+node_modules/@wordpress/create-block-interactive-template
node_modules/@wordpress/create-block-tutorial-template
node_modules/@wordpress/create-block
node_modules/@wordpress/customize-widgets
@@ -240,3 +241,176 @@ node_modules/webdriverio
node_modules/webpack-bundle-analyzer
node_modules/webpack
node_modules/worker-farm
+node_modules/@babel/runtime
+packages/annotations/node_modules/uuid
+node_modules/deepmerge
+node_modules/gettext-parser
+node_modules/is-plain-object
+node_modules/@babel/plugin-transform-react-jsx
+node_modules/@babel/plugin-transform-runtime
+node_modules/@babel/preset-env
+node_modules/@babel/preset-typescript
+node_modules/core-js
+node_modules/clsx
+node_modules/@emotion/react
+node_modules/@emotion/styled
+node_modules/@react-spring/web
+node_modules/colord
+node_modules/diff
+node_modules/fast-deep-equal
+node_modules/memize
+node_modules/parsel-js
+node_modules/postcss-prefix-selector
+packages/block-editor/node_modules/postcss-urlrebase
+node_modules/react-autosize-textarea
+packages/block-editor/node_modules/react-easy-crop
+node_modules/remove-accents
+node_modules/fast-average-color
+packages/block-library/node_modules/uuid
+node_modules/pegjs
+node_modules/phpegjs
+node_modules/hpq
+packages/blocks/node_modules/react-is
+node_modules/showdown
+node_modules/simple-html-tokenizer
+packages/blocks/node_modules/uuid
+packages/commands/node_modules/cmdk
+packages/components/node_modules/@ariakit/react
+node_modules/@emotion/cache
+node_modules/@emotion/css
+node_modules/@emotion/serialize
+node_modules/@emotion/utils
+packages/components/node_modules/@floating-ui/react-dom
+packages/components/node_modules/@types/gradient-parser
+node_modules/@types/highlight-words-core
+node_modules/@use-gesture/react
+node_modules/date-fns
+node_modules/framer-motion
+node_modules/gradient-parser
+node_modules/highlight-words-core
+packages/components/node_modules/path-to-regexp
+node_modules/re-resizable
+node_modules/react-colorful
+packages/components/node_modules/uuid
+node_modules/@types/mousetrap
+packages/compose/node_modules/clipboard
+node_modules/mousetrap
+node_modules/use-memo-one
+packages/core-data/node_modules/uuid
+node_modules/check-node-version
+node_modules/mustache
+node_modules/npm-package-arg
+node_modules/write-pkg
+node_modules/is-promise
+node_modules/rememo
+packages/dataviews/node_modules/@ariakit/react
+node_modules/moment-timezone
+node_modules/moment
+node_modules/json2php
+node_modules/comment-parser
+node_modules/mdast-util-inject
+node_modules/optionator
+node_modules/remark-parse
+node_modules/remark
+node_modules/unified
+node_modules/form-data
+node_modules/get-port
+node_modules/lighthouse
+node_modules/mime
+packages/e2e-test-utils-playwright/node_modules/web-vitals
+node_modules/expect-puppeteer
+node_modules/jest-snapshot
+packages/e2e-tests/node_modules/uuid
+node_modules/copy-dir
+packages/env/node_modules/docker-compose
+node_modules/extract-zip
+node_modules/got
+node_modules/js-yaml
+node_modules/ora
+node_modules/terminal-link
+packages/env/node_modules/yargs
+node_modules/@babel/eslint-parser
+node_modules/@typescript-eslint/eslint-plugin
+node_modules/@typescript-eslint/parser
+node_modules/cosmiconfig
+node_modules/eslint-config-prettier
+node_modules/eslint-plugin-jsdoc
+node_modules/eslint-plugin-jsx-a11y
+node_modules/eslint-plugin-playwright
+node_modules/eslint-plugin-react-hooks
+node_modules/eslint-plugin-react
+node_modules/eslint
+node_modules/globals
+node_modules/requireindex
+node_modules/tannin
+packages/interactivity/node_modules/@preact/signals
+packages/interactivity/node_modules/preact
+node_modules/jest-matcher-utils
+node_modules/@axe-core/puppeteer
+node_modules/npm-package-json-lint
+node_modules/autoprefixer
+node_modules/requestidlecallback
+node_modules/@octokit/request-error
+packages/project-management-automation/node_modules/@octokit/webhooks
+node_modules/utility-types
+node_modules/@react-native-clipboard/clipboard
+node_modules/@react-native-community/blur
+node_modules/@react-native-community/slider
+node_modules/@react-native-masked-view/masked-view
+node_modules/@react-navigation/core
+node_modules/@react-navigation/native
+node_modules/@react-navigation/routers
+node_modules/@react-navigation/stack
+node_modules/jed
+node_modules/jsdom-jscore-rn
+node_modules/react-native-fast-image
+node_modules/react-native-gesture-handler
+node_modules/react-native-get-random-values
+node_modules/react-native-linear-gradient
+node_modules/react-native-modal
+node_modules/react-native-reanimated
+node_modules/react-native-safe-area-context
+node_modules/react-native-safe-area
+node_modules/react-native-sass-transformer
+node_modules/react-native-screens
+node_modules/react-native-svg
+packages/react-native-editor/node_modules/react-native-url-polyfill
+node_modules/react-native-video
+node_modules/react-native-webview
+node_modules/rungen
+packages/report-flaky-tests/node_modules/@actions/github
+node_modules/history
+node_modules/@svgr/webpack
+node_modules/adm-zip
+packages/scripts/node_modules/babel-loader
+packages/scripts/node_modules/chalk
+node_modules/clean-webpack-plugin
+node_modules/cross-spawn
+node_modules/cwd
+node_modules/dir-glob
+packages/scripts/node_modules/jest-dev-server
+node_modules/jest-environment-node
+node_modules/markdownlint-cli
+node_modules/merge-deep
+node_modules/mini-css-extract-plugin
+node_modules/minimist
+node_modules/npm-packlist
+node_modules/read-pkg-up
+node_modules/resolve-bin
+node_modules/rtlcss-webpack-plugin
+packages/scripts/node_modules/schema-utils
+node_modules/stylelint
+node_modules/url-loader
+node_modules/webpack-cli
+node_modules/webpack-dev-server
+node_modules/@stylistic/stylelint-plugin
+packages/stylelint-config/node_modules/stylelint-config-recommended-scss
+packages/stylelint-config/node_modules/stylelint-config-recommended
+node_modules/@types/simple-peer
+node_modules/import-locals
+node_modules/lib0
+node_modules/simple-peer
+node_modules/y-indexeddb
+node_modules/y-protocols
+node_modules/y-webrtc
+node_modules/yjs |
|
Here's a test, I added this package at {
"name": "test-package",
"version": "1.0.0",
"main": "index.js",
"scripts": {
"test": "echo \"Error: no test specified\" && exit 1"
},
"author": "",
"license": "ISC",
"description": "",
"devDependencies": {
"ansi-align": "3.0.1"
}
}That's a new dependency. It's a dev dependency in the root. This is part of the updated package-lock: |
|
Of course, the more interesting test is that a new prod dependency of any package in the workspace is a prod dependency in the root package json: "description": "",
- "devDependencies": {
+ "dependencies": {
"ansi-align": "3.0.1"
}
} "ansi-align": {
"version": "3.0.1",
"resolved": "https://registry.npmjs.org/ansi-align/-/ansi-align-3.0.1.tgz",
"integrity": "sha512-IOfwwBF5iczOjp/WeY4YxyjqAFMQoZufdQWDd19SEExbVLNXqvpzSJ/M7Za4/sCPmQ0+GRquoA7bGcINcxew6w==",
- "dev": true,This does seem to indicate that the root package-lock functions as a union of workspace dependencies. This may be related to hoisting and other behaviors, but I think the behavior observed in this PR where many |
|
I created #65708 (targeting this PR) with some ideas. I think we may want to move away from file refs now that we have workspaces. Lerna seems to handle it correctly in my testing. |
|
Thanks for the detailed investigation! If I understand correctly, the libraries that are causing a lot of Please let me know if I can help you with anything. |
|
If we change to using workspaces —and I think we should— then it's really a rethinking of how the project is organized. There will be some learning along the way.
I don't think this needs to happen, but it seems to me that the root project effectively functions as an aggregate of all of the workspaces it contains. All of the workspaces are installed in the root I think it would be good to wait until this release is settled (to prevent disruption at a critical time) and then explore some of these changes. |
That's a good point. It seems like a correct assumption that all WP packages would be now in the workspace so we no longer need to reference them from the root The development packages should continue to be maintained from the root of the project as explained https://github.com/WordPress/gutenberg/tree/trunk/packages#development-dependencies. The biggest challenge moving forward is going to be how to configure |
I was curious about this and it was something I explored in #65708. Lerna's docs use We should definitely do more testing there. |
That would work assuming that |
|
I personally don't have many insights, apart from the fact that currently updating dependencies is not a simple task. The only reliable way, for me, is to remove the dependencies, run So any changes that can improve the process would be great! |
|
I tried to figure out why all the But now these packages are included by virtue of being in So, the disappearance of |
|
Few other things we should probably do when enabling npm workspaces for the repo: Migrate calls of After this, we'll be using Lerna only for the publishing workflow: Remove individual |
This is how it should be. Lerna is only useful for npm publishing these days. I don't know if still the best tool for the job, but the amount of work necessary to migrate to any other tool is probably not worth it. I personally won't have availability to help with such migration in the near future.
This flag currently prevents creating lock files in each individual package which is no longer necessary with workspaces as it uses a single lock file. You simply don't need these lock files with monorepo so they would only create noise. |
While it's perfectly fine. It means we will have to fix the script that checks licenses. I think the simplest way will be ignoring packages that are used only for dev purposes, similar to how it happens today with some packages: Line 308 in ec5acbd Example for checking production: wp-scripts check-licenses --prod --gpl2 --ignore=@wordpress/scripts,@wordpress/babel-preset-default,@wordpress/...It isn't entirely clear how the same can be achieved for dev packages that will be marked now as production packages, maybe: wp-scripts check-licenses --prod --ignore=@wordpress/block-edtior,@wordpress/blocks,@wordpress/...In effect, we might need three parallel scripts moving forward. It's also possible that |
We want to check licenses only for packages that are going to be included in the Gutenberg plugin, right? Not for other ones that are merely published to NPM. This is very tightly coupled to how the packages are listed and built in Now when the top-level dependencies are going away, replaced by workplaces, we need to find a new place for this source of truth -- where the production |
|
With Gutenberg 19.4 recently released, It should be an ideal time to work on this. #65708 is likely a good start. |
this should be done another way
…ng npm dependencies from packages
sirreal
force-pushed
the
managing-packages
branch
from
d10c7d6 to
770ab08
Compare
Refactor some of the check-license logic into a reusable module. The check-license script continues to act as the CLI, while another module contains the license checking logic. This is helpful for work migrating to npm workspaces: WordPress#65681 The license checking logic is required, but the implementation will be different to account for npm workspaces in Gutenberg. --- Co-authored-by: sirreal <[email protected]> Co-authored-by: gziolo <[email protected]>
Update package management to use npm workspaces instead of lerna. Lerna is obsolete for many repository management operations, such as dependency installation or script running. All of these are supported by npm now. Lerna continues to be used for publishing flows to ensure the appropriate dependencies are updated and published with the correct versions. The hope is that this simplfies dependendy management and maintains a more sane dependency tree. Dependency management has become increasingly difficult as Gutenberg has grown. --- Co-authored-by: t-hamano <[email protected]> Co-authored-by: sirreal <[email protected]> Co-authored-by: gziolo <[email protected]> Co-authored-by: ciampo <[email protected]> Co-authored-by: jsnajdr <[email protected]> Co-authored-by: yusuke-omae <[email protected]> Co-authored-by: kevin940726 <[email protected]>
) This reverts commit 4693c0f.
See this Slack comment: https://wordpress.slack.com/archives/C02QB2JS7/p1727315839205439
What?
This PR adds npm workspaces and updates the documentation to allow existing dependencies to be properly updated.
Why?
lerna has a
lerna addcommand to add dependencies to the monorepo. However, since npm native now has a similar feature, this command was removed in version 7. Therefore, the current documented method cannot properly update dependencies.How?
For npm publishing, we will continue to use lerna as before. This PR allows us to run npm native commands that are an alternative to
lerna addby adding aworkspacestopackage.json.lerna will refer to
workspacesinpackage.jsonby default, but since the settings inlerna.jsontake precedence, this should not affect npm publishing (Source).Testing Instructions
Please run a command like the following to make sure the dependencies are added properly:
The script modules webpack build config should be identical. This can be verified by adding some logging and executing the file
node ./tools/webpack/script-modules.js:Closes #59933.