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
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ jobs:
uses: actions/checkout@ec3a7ce113134d7a93b817d10a8272cb61118579 # v2.4.0

- name: Install NodeJS
uses: actions/setup-node@5b52f097d36d4b0b2f94ed6de710023fbb8b2236 # v3.1.0
uses: actions/setup-node@eeb10cff27034e7acf239c5d29f62154018672fd # v3.3.0
with:
node-version-file: '.nvmrc'
cache: 'npm'
cache: yarn

- name: Install all dependencies
run: |
Expand Down
2 changes: 1 addition & 1 deletion .nvmrc
Original file line number Diff line number Diff line change
@@ -1 +1 @@
14
20
34 changes: 20 additions & 14 deletions bin/build.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,23 +13,26 @@ const { sync: spawn } = require( 'cross-spawn' );
const postCssConfig = require( '../postcss.config.js' );

/**
* Build the JS files using webpack, if the `src` directory exists.
* Build the files, if the `src` directory exists.
*
* This builds JS and any SCSS, using wp-scripts. We can't use wp-scripts
* directly due to the folder structure of the blocks, but can pass through
* the folders with CLI args.
*
* @param {string} inputDir
* @param {string} outputDir
*/
async function maybeBuildJavaScript( inputDir, outputDir ) {
async function maybeBuildBlock( inputDir, outputDir ) {
const project = path.basename( path.dirname( inputDir ) );
if ( fs.existsSync( inputDir ) ) {
// Set the src directory based on the relative location from projet root.
process.env.WP_SRC_DIRECTORY = path.relative( path.dirname( __dirname ), inputDir );
const { status, stdout } = spawn(
resolveBin( 'webpack' ),
// Run wp-scripts with a specific input and output directory.
const { status, output } = spawn(
resolveBin( '@wordpress/scripts', { executable: 'wp-scripts' } ),
[
'--config',
path.join( path.dirname( __dirname ), 'webpack.config.js' ),
'--output-path',
outputDir,
'build',
'--experimental-modules',
'--webpack-src-dir=' + path.relative( path.dirname( __dirname ), inputDir ),
'--output-path=' + outputDir,
'--color', // Enables colors in `stdout`.
],
{
Expand All @@ -38,10 +41,10 @@ async function maybeBuildJavaScript( inputDir, outputDir ) {
);
// Only output the webpack result if there was an issue.
if ( 0 !== status ) {
console.log( stdout.toString() );
console.log( chalk.red( `Error in JavaScript for ${ project }` ) );
console.log( output.toString() );
console.log( chalk.red( `Error in block for ${ project }` ) );
} else {
console.log( chalk.green( `JavaScript built for ${ project }` ) );
console.log( chalk.green( `Block built for ${ project }` ) );
}
}
}
Expand Down Expand Up @@ -109,7 +112,10 @@ projects.forEach( async ( file ) => {

// We `await` because JS needs to be built first— the first webpack step deletes the build
// directory, and could remove the built CSS if it was truely async.
await maybeBuildJavaScript( path.resolve( path.join( basePath, 'src' ) ), outputDir );
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.

await maybeBuildPostCSS( path.resolve( path.join( basePath, 'postcss' ) ), outputDir );
} catch ( error ) {
console.log( chalk.red( `Error in ${ file }:` ), error.message );
Expand Down
2 changes: 1 addition & 1 deletion bin/watch.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ function isSourceFile( filename ) {
const relativePath = path.relative( process.cwd(), filename ).replace( /\\/g, '/' );

return (
[ /\/src\/.+\.(js|json|ts|tsx)$/, /\/postcss\/.+\.(pcss)$/ ].some( ( regex ) =>
[ /\/src\/.+\.(js|json|ts|tsx|scss)$/, /\/postcss\/.+\.(pcss)$/ ].some( ( regex ) =>
regex.test( relativePath )
) && ! [ /\/__tests__\/.+/, /.\.(spec|test)\.js$/ ].some( ( regex ) => regex.test( relativePath ) )
);
Expand Down
8 changes: 0 additions & 8 deletions mu-plugins/blocks/horizontal-slider/package.json

This file was deleted.

2 changes: 1 addition & 1 deletion mu-plugins/blocks/language-suggest/src/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { useBlockProps } from '@wordpress/block-editor';
* The edit function describes the structure of your block in the context of the
* editor. This represents what the editor will render when the block is used.
*
* @return {WPElement} Element to render.
* @return {Element} Element to render.
*/
export default function Edit() {
return (
Expand Down
2 changes: 1 addition & 1 deletion mu-plugins/blocks/language-suggest/src/save.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { useBlockProps } from '@wordpress/block-editor';
* be combined into the final markup, which is then serialized by the block
* editor into `post_content`.
*
* @return {WPElement} Element to render.
* @return {Element} Element to render.
*/
export default function save() {
return <div { ...useBlockProps.save() }></div>;
Expand Down
2 changes: 1 addition & 1 deletion mu-plugins/blocks/latest-news/src/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import ServerSideRender from '@wordpress/server-side-render';
* @param {Function} props.setAttributes
* @param {string} props.name
*
* @return {WPElement} Element to render.
* @return {Element} Element to render.
*/
export default function Edit( { attributes, setAttributes, name } ) {
const { blogId, perPage, showCategories } = attributes;
Expand Down
2 changes: 1 addition & 1 deletion mu-plugins/blocks/sidebar-container/src/view.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ function getCustomPropValue( name, element = document.body ) {
function onScroll() {
// Only run the scroll code if the sidebar is floating on a wide screen.
if ( ! mainEl || ! container || ! window.matchMedia( '(min-width: 1200px)' ).matches ) {
return;
return false;
}

const scrollPosition = window.scrollY - ADMIN_BAR_HEIGHT;
Expand Down
27 changes: 13 additions & 14 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,28 +12,27 @@
"url": "https://github.com/WordPress/wporg-mu-plugins/issues"
},
"devDependencies": {
"@wordpress/browserslist-config": "4.1.2",
"@wordpress/env": "5.5.0",
"@wordpress/scripts": "23.5.0",
"@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"
Comment on lines +15 to +28
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.

},
"browserslist": [
"extends @wordpress/browserslist-config"
],
"scripts": {
"build": "NODE_ENV=production ./bin/build.js",
"build:dev": "NODE_ENV=development ./bin/build.js",
"font-subset": "./bin/font-subset.js",
"prestart": "./bin/build.js",
"start": "NODE_ENV=development ./bin/watch.js",
Expand All @@ -42,7 +41,7 @@
"setup:tools": "yarn install && composer install && TEXTDOMAIN=wporg composer exec update-configs",
"update:tools": "composer update && TEXTDOMAIN=wporg composer exec update-configs",
"wp-env": "wp-env",
"test:php": "wp-env run phpunit 'phpunit -c /var/www/html/phpunit.xml.dist --verbose'"
"test:php": "wp-env run tests-cli --env-cwd=/var/www/html/ phpunit"
},
"rtlcssConfig": {},
"stylelint": {
Expand Down
3 changes: 0 additions & 3 deletions webpack.config.js

This file was deleted.

Loading
Loading