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.

Sign up for GitHub

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

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d1d1780
Tools: Bump node version, JS dependency versions
ryelle Jan 25, 2024
a9c4f13
Tools: Update github build action
ryelle Jan 25, 2024
8d7f7a5
Code style: Update to fix linter issues
ryelle Jan 25, 2024
5a4d7c1
Tools: Update build process to use wp-scripts
ryelle Jan 25, 2024
9ca0663
Tools: Update test command to use new server name
ryelle Jan 26, 2024
c79c8af
Remove comment about deprecated PostCSS
ryelle Jan 29, 2024
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
34 changes: 20 additions & 14 deletions bin/build.js
View file
Open in desktop
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
View file
Open in desktop
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
5 changes: 2 additions & 3 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,14 @@
"postcss-preset-env": "9.3.0",
"resolve-bin": "1.0.1",
"rtlcss": "4.1.1",
"url-loader": "4.1.1",
"webpack": "5.90.0",
"webpack-cli": "5.1.4"
"url-loader": "4.1.1"
},
"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 Down
3 changes: 0 additions & 3 deletions webpack.config.js
View file
Open in desktop

This file was deleted.

4 changes: 2 additions & 2 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10772,7 +10772,7 @@ webpack-bundle-analyzer@^4.9.1:
sirv "^2.0.3"
ws "^7.3.1"

webpack-cli@5.1.4, webpack-cli@^5.1.4:
webpack-cli@^5.1.4:
version "5.1.4"
resolved "https://registry.yarnpkg.com/webpack-cli/-/webpack-cli-5.1.4.tgz#c8e046ba7eaae4911d7e71e2b25b776fcc35759b"
integrity sha512-pIDJHIEI9LR0yxHXQ+Qh95k2EvXpWzZ5l+d+jIo+RdSm9MiHfzazIxwwni/p7+x4eJZuvG1AJwgC4TNQ7NRgsg==
Expand Down Expand Up @@ -10852,7 +10852,7 @@ webpack-sources@^3.2.3:
resolved "https://registry.npmjs.org/webpack-sources/-/webpack-sources-3.2.3.tgz"
integrity sha512-/DyMEOrDgLKKIG0fmvtz+4dUX/3Ghozwgm6iPp8KRhvn+eQf9+Q7GWxVNMk3+uCPWfdXYC4ExGBckIXdFEfH1w==

webpack@5.90.0, webpack@^5.88.2:
webpack@^5.88.2:
version "5.90.0"
resolved "https://registry.yarnpkg.com/webpack/-/webpack-5.90.0.tgz#313bfe16080d8b2fee6e29b6c986c0714ad4290e"
integrity sha512-bdmyXRCXeeNIePv6R6tGPyy20aUobw4Zy8r0LUS2EWO+U+Ke/gYDgsCh7bl5rB6jPpr4r0SZa6dPxBxLooDT3w==
Expand Down
Loading