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

Build Tooling: Optimize build by spawning worker pool #15230

Merged
merged 7 commits into from
May 30, 2019
Merged
Show file tree
Hide file tree
Changes from all 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
159 changes: 159 additions & 0 deletions bin/packages/build-worker.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
/**
* External dependencies
*/
const { promisify } = require( 'util' );
const fs = require( 'fs' );
const path = require( 'path' );
const babel = require( '@babel/core' );
const makeDir = require( 'make-dir' );
const sass = require( 'node-sass' );
const postcss = require( 'postcss' );

/**
* Internal dependencies
*/
const getBabelConfig = require( './get-babel-config' );

/**
* Path to packages directory.
*
* @type {string}
*/
const PACKAGES_DIR = path.resolve( __dirname, '../../packages' );

/**
* Mapping of JavaScript environments to corresponding build output.
*
* @type {Object}
*/
const JS_ENVIRONMENTS = {
main: 'build',
module: 'build-module',
};

/**
* Promisified fs.readFile.
*
* @type {Function}
*/
const readFile = promisify( fs.readFile );

/**
* Promisified fs.writeFile.
*
* @type {Function}
*/
const writeFile = promisify( fs.writeFile );

/**
* Promisified sass.render.
*
* @type {Function}
*/
const renderSass = promisify( sass.render );

/**
* Get the package name for a specified file
*
* @param {string} file File name
* @return {string} Package name
*/
function getPackageName( file ) {
return path.relative( PACKAGES_DIR, file ).split( path.sep )[ 0 ];
}

/**
* Get Build Path for a specified file.
*
* @param {string} file File to build
* @param {string} buildFolder Output folder
* @return {string} Build path
*/
function getBuildPath( file, buildFolder ) {
const pkgName = getPackageName( file );
const pkgSrcPath = path.resolve( PACKAGES_DIR, pkgName, 'src' );
const pkgBuildPath = path.resolve( PACKAGES_DIR, pkgName, buildFolder );
const relativeToSrcPath = path.relative( pkgSrcPath, file );
return path.resolve( pkgBuildPath, relativeToSrcPath );
}

/**
* Object of build tasks per file extension.
*
* @type {Object<string,Function>}
*/
const BUILD_TASK_BY_EXTENSION = {
async '.scss'( file ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

😮I'd never considered this as a way to define object properties. Very fancy!

const outputFile = getBuildPath( file.replace( '.scss', '.css' ), 'build-style' );
const outputFileRTL = getBuildPath( file.replace( '.scss', '-rtl.css' ), 'build-style' );

const [ , contents ] = await Promise.all( [
makeDir( path.dirname( outputFile ) ),
readFile( file, 'utf8' ),
] );

const builtSass = await renderSass( {
file,
includePaths: [ path.resolve( __dirname, '../../assets/stylesheets' ) ],
data: (
[
'colors',
'breakpoints',
'variables',
'mixins',
'animations',
'z-index',
].map( ( imported ) => `@import "${ imported }";` ).join( ' ' ) +
contents
),
} );

const result = await postcss( require( './post-css-config' ) ).process( builtSass.css, {
from: 'src/app.css',
to: 'dest/app.css',
} );

const resultRTL = await postcss( [ require( 'rtlcss' )() ] ).process( result.css, {
from: 'src/app.css',
to: 'dest/app.css',
} );

await Promise.all( [
writeFile( outputFile, result.css ),
writeFile( outputFileRTL, resultRTL.css ),
] );
},

async '.js'( file ) {
for ( const [ environment, buildDir ] of Object.entries( JS_ENVIRONMENTS ) ) {
const destPath = getBuildPath( file, buildDir );
const babelOptions = getBabelConfig( environment, file.replace( PACKAGES_DIR, '@wordpress' ) );

const [ , transformed ] = await Promise.all( [
makeDir( path.dirname( destPath ) ),
babel.transformFileAsync( file, babelOptions ),
] );

await Promise.all( [
writeFile( destPath + '.map', JSON.stringify( transformed.map ) ),
writeFile( destPath, transformed.code + '\n//# sourceMappingURL=' + path.basename( destPath ) + '.map' ),
] );
}
},
};

module.exports = async ( file, callback ) => {
const extension = path.extname( file );
const task = BUILD_TASK_BY_EXTENSION[ extension ];
Copy link
Member Author

@aduth aduth May 29, 2019

Choose a reason for hiding this comment

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

Noting for future: I've been planning in my head how I might implement something like #13124 . I don't plan to make the changes here, since it's already a fairly significant refactor.

I expect this task could be separated out into a function buildFile, which is optionally enhanced to read from / write to a cache. I'm thinking in some cases we may want to opt out of the caching behavior, e.g. in npm run package-plugin, since it seems there could be a (very-low-probability-but-possible) chance that hash collisions could yield a bad result†. That said, we should always be testing (manually and automated) the artifact, but as of today, we're only doing the former.

let buildFile = ( file ) => { /* ... */ };

if ( hasCache ) {
	buildFile = firstMatch( [ fromCache, buildFile ] );
}

buildFile = flow( [ buildFile, toCache ] );

† This assumes that an implementation would be similar to babel-loader in checking a cache folder for a file named by the MD5 (or other fast hash) of the source file's content.

cc @noisysocks

Copy link
Member

@noisysocks noisysocks May 30, 2019

Choose a reason for hiding this comment

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

Perhaps store the cache in a directory within the repository that is .gitignored? That way, when npm run package-plugin runs git clean -fxd it will empty the cache.

Not sure I'd worry about collisions. I'd use SHA1 as it's fast, has a low collision rate, and is good enough for git's object storage model which what inspired my approach in #13124.

Copy link
Member Author

Choose a reason for hiding this comment

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

Perhaps store the cache in a directory within the repository that is .gitignored? That way, when npm run package-plugin runs git clean -fxd it will empty the cache.

That's true, I'd not considered that. My plan was to do similar to what's done with babel-loader, i.e. a folder at node_modules/.cache. Seems it would work just the same as you propose (since node_modules is gitignored).

Not sure I'd worry about collisions. I'd use SHA1 as it's fast, has a low collision rate, and is good enough for git's object storage model which what inspired my approach in #13124.

I'm not overly worried, I'll just inevitably feel uncomfortable about the lack of being able to say that it's "impossible". I don't feel too strongly about the specific hash algorithm, just that it's fast. Some of what I've read seemed to imply MD5 beats out SHA1 on this front 🤷‍♂ [1][2][3]


if ( ! task ) {
return;
}

try {
await task( file );
callback();
} catch ( error ) {
callback( error );
}
};
Loading