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

Tools: Update packages, use wp-scripts for build process #562

Merged
merged 6 commits into from
Jan 29, 2024

Conversation

ryelle
Copy link
Contributor

@ryelle ryelle commented Jan 26, 2024

The Interactivity API is now only available as a JS module, not a plain script, so we need to build and register our view code differently. As of v27, @wordpress/scripts supports module builds, but that also breaks compat with Node <18. So this PR updates the node version to match core, and updates the rest of the JS packages.

I've also changed the build process to call wp-scripts directly for each block, rather than using webpack + importing the wp-scripts webpack config. The block build step is now equivalent to running:

npx wp-scripts build --experimental-modules --webpack-src-dir=./mu-plugins/blocks/[block]/src --output-path=./mu-plugins/blocks/[block]/build

Unfortunately due to the folder structure, we still need the custom script (wp-scripts doesn't understand how to find [block]/src/block.json, it only understands src/[block]/block.json).

See WordPress/wporg-parent-2021#122, WordPress/gutenberg#56143, WordPress/gutenberg#57461

To test

  • Run nvm install to switch the node version (or however you manage node versions), it should be 20.
  • Run yarn to install the package updates
    • You might need to remove node_modules first, I had a strange issue with wp-scripts not updating correctly
  • Run yarn build
    • All builds should succeed, both CSS and JS
    • View any file, it should be minified, ex: cat mu-plugins/blocks/navigation/build/index.js
  • Run the new yarn build:dev
    • All builds should succeed
    • View any file, it should not be minified, ex: cat mu-plugins/blocks/navigation/build/index.js
  • Run the build steps with a block, ex yarn build:dev query-filter
  • Run the watch script
    • The initial build of all modules should succeed
    • Changing any files in blocks, JS, CSS, etc, should trigger a rebuild of the files

@ryelle ryelle force-pushed the update/tools-packages-build branch from 51b8e39 to 9ca0663 Compare January 26, 2024 18:20
@ryelle ryelle marked this pull request as ready for review January 26, 2024 21:16
Copy link
Contributor

@renintw renintw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Followed the test steps, and all worked as expected. Thanks for the update👍

bin/build.js Outdated
await maybeBuildBlock( path.resolve( path.join( basePath, 'src' ) ), outputDir );

// `maybeBuildBlock` supports SCSS, but current blocks still have postCSS
// files. Until those are converted, we still need a separate PostCSS step.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So will we eventually phase out all PostCSS in favor of SCSS? I'm not sure if the discussion here is relevant because it seems to be discussing the opposite idea.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, thanks for pointing me to that conversation, it happened while I was away :)

Honestly I was remembering this recent bug where the CSS was not building properly because it was using Sass formatting. I didn't realize there had been a more recent discussion than the 2022 issue.

… turns out though, wp-scripts does support *.pcss, so we could remove this without forcing a Sass decision. I'll update the comment here and try that in a new PR.

Comment on lines +15 to +28
"@wordpress/browserslist-config": "5.33.0",
"@wordpress/env": "9.2.0",
"@wordpress/scripts": "27.1.0",
"chalk": "4.1.2",
"cross-spawn": "7.0.3",
"cssnano": "5.1.11",
"node-watch": "^0.7.3",
"postcss": "8.4.14",
"postcss-cli": "9.1.0",
"postcss-import": "14.1.0",
"postcss-preset-env": "7.7.1",
"cssnano": "6.0.3",
"node-watch": "0.7.4",
"postcss": "8.4.33",
"postcss-cli": "11.0.0",
"postcss-import": "16.0.0",
"postcss-preset-env": "9.3.0",
"resolve-bin": "1.0.1",
"rtlcss": "3.5.0",
"url-loader": "^3.0.0",
"webpack": "5.47.1",
"webpack-cli": "4.9.1"
"rtlcss": "4.1.1",
"url-loader": "4.1.1"
Copy link
Contributor

@adamwoodnz adamwoodnz Jan 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How did you arrive at this set of dependencies? The changes seem more extensive than expected.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used yarn outdated to update everything available. I wanted to make sure any peer dependencies were up to date with the new wordpress/ packages & supported the latest node version.

Copy link
Contributor

@adamwoodnz adamwoodnz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Works for me too, thanks!

@ryelle
Copy link
Contributor Author

ryelle commented Jan 29, 2024

I'm going to merge this, since it doesn't require a deploy & isn't affected by a site's Gutenberg version.

@ryelle ryelle merged commit 0169ab4 into trunk Jan 29, 2024
2 checks passed
@ryelle ryelle deleted the update/tools-packages-build branch January 29, 2024 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants