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: Dynamically pick JS/CSS build files for ZIP gen #5664

Merged
merged 1 commit into from
Mar 20, 2018

Conversation

mcsf
Copy link
Contributor

@mcsf mcsf commented Mar 16, 2018

Description

Sparked by #5430 (comment), this aims to reduce the need for manual synchronization between webpack.config.js, bin/build-plugin-zip.sh, .eslintrc.js, and any other place where knowledge of which entry points Gutenberg has is needed.

To keep it short, this PR only targets bin/build-plugin-zip.sh and uses the shell's ** file expansion.

Previously

An initial exploration saw the creation of a small bin/list-entry-points.js script which read webpack.config.js (thus treating it as the canonical source for entry-point information) and roughly did:

$entry_points=$(bin/list-entry-points.js | tr \\n , | sed 's/,$//') # comma-separated list
$build_files=$(eval echo {$entry_points}/build/*.{js,css) # actually wrong, because `*.css* will emerge whenever a proper CSS file is missing
#!/usr/bin/env node
// For reference, here's bin/list-entry-points.js

const config = require( '../webpack.config.js' );

Object.keys( config.externals )
	// Filter out other externals, keep entry points
	.filter( k => k.indexOf( '@wordpress/' ) >= 0 )
	// Remove the '@wordpress/' prefix
	.map( k => k.substr( 11 ) )
	// Fix key 'editPost,edit-post'
	.map( k => k.indexOf( 'editPost' ) >= 0 ? 'edit-post' : k )
	.forEach( k => console.log( k ) );

How Has This Been Tested?

Run ./bin/build-plugin-zip.sh. Make sure no errors are found. Inspect the generated ZIP file. Compare its manifesto (e.g. unzip -l <file>) with the one from the latest release or one generated from master.

Screenshots (jpeg or gifs if applicable):

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code has proper inline documentation.

@mcsf mcsf added the [Type] Build Tooling Issues or PRs related to build tooling label Mar 16, 2018
@mcsf mcsf mentioned this pull request Mar 16, 2018
3 tasks
@mcsf mcsf requested a review from youknowriad March 19, 2018 14:47
@mcsf mcsf changed the title Build: Dynamically use JS/CSS build files for ZIP gen Build: Dynamically pick JS/CSS build files for ZIP gen Mar 19, 2018
@@ -99,8 +101,7 @@ zip -r gutenberg.zip \
blocks/library/*/*.php \
post-content.js \
$vendor_scripts \
{blocks,components,date,editor,element,hooks,i18n,data,utils,edit-post,viewport}/build/*.{js,map} \
Copy link
Contributor Author

@mcsf mcsf Mar 19, 2018

Choose a reason for hiding this comment

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

I don't believe the *.{js,map} bit was up-to-date. In any case, we don't expect .map files in the ZIP file, to my knowledge. Hence my removal.

@aduth aduth force-pushed the add/dynamic-build-file-bundling branch from 7a98897 to ec18afa Compare March 20, 2018 01:18
Copy link
Member

@aduth aduth left a comment

Choose a reason for hiding this comment

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

👍

Rebased to resolve conflicts.

While this makes bundling less intentional (we were previously targeting specific folders where we expect CSS to exist), I think it lends to better overall maintainability.

@mcsf
Copy link
Contributor Author

mcsf commented Mar 20, 2018

While this makes bundling less intentional (we were previously targeting specific folders where we expect CSS to exist)

Agreed. I can see this sort of work progress into clearly defining our entry points and leveraging said definition throughout the project.

@mcsf mcsf merged commit 1059cd2 into master Mar 20, 2018
@mcsf mcsf deleted the add/dynamic-build-file-bundling branch March 20, 2018 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Build Tooling Issues or PRs related to build tooling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants