-
Notifications
You must be signed in to change notification settings - Fork 893
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
Upgrade for node v18 #20839
Upgrade for node v18 #20839
Conversation
d3b59fd
to
b9cacc5
Compare
The UI library tests are failing with node 20
Pull Request Test Coverage Report for Build 1c577ba9c975592063e97de03292c00eea9e6c73
💛 - Coveralls |
… and use it in CI and release prosessing
Using the 2 spaces as specified in editorconfig Rewording the comments a bit
b938de6
to
47d3396
Compare
This is never going to get a nice doc comment. And I would like to trigger the yoastseo tests on the CI :)
it seems to be happening already on |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok for me
…upgrade-for-node # Conflicts: # yarn.lock
By doing the same trick again as in #21055 Without this, the UI library build can not find `@babel/helper-create-regexp-features-plugin`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me too! ✅
Context
Node LTS compatibility is the goal.
Not only that, but also adding some structure to have the code as source of truth for future version changes. The idea is that this will make it easier to test and thus upgrade further.
Added bonus is us streamlining the version we use locally to also be used in our CI and releasing.
Summary
This PR can be summarized in the following changelog entry:
@wordpress/scripts
and@wordpress/dependency-extraction-webpack-plugin
, for their compatibility with Node v18.MiniCssExtractPlugin
usage in our Webpack configuration, not being removed from default config and updating to passing a relative path. This was needed after the upgrade of WP scripts..nvmrc
, and use that in our GH-actions.VERSIONS.json
file that holds information about our used versions for Composer, Node, PHP and Yarn. Though Composer and PHP are the only ones currently read from. Node and Yarn are specified in.nvmrc
and.yarnrc
, respectively.Relevant technical choices:
.nvmrc
as source of truth for the Node version as that syncs up with development..yarnrc
as source of truth for the Yarn version, because that is how Yarn enforces it (it turns out the brief Yarn upgrade did not work in the first place).VERSIONS.json
is an implementation from our Devops that was used in our platform already. It is currently implemented on a test branch of our CI. And currently has a test path in there to pull in this branch, for testing purposes.Test instructions
Test instructions for the acceptance test before the PR gets merged
This PR can be acceptance tested by following these steps:
Locally [devs only]
PR build
RC/Release
Arnoud made a branch on our CI that pulls in this branch and reads from
VERSIONS.json
. This makes it so we can run the PRE/test versions of our RC and release scripts on this branch.UI library Storybook [devs only]
yarn workspace @yoast/ui-library storybook
)Relevant test scenarios
Test instructions for QA when the code is in the RC
QA can test this PR by following these steps:
Impact check
This PR affects the following parts of the plugin, which may require extra testing:
UI changes
Other environments
[shopify-seo]
, added test instructions for Shopify and attached theShopify
label to this PR.Documentation
Quality assurance
Innovation
innovation
label.Fixes #19565