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

Re-use webpack and babel config for build script #12685

Closed
wants to merge 16 commits into from
Closed

Re-use webpack and babel config for build script #12685

wants to merge 16 commits into from

Conversation

griffbrad
Copy link

@griffbrad griffbrad commented Dec 7, 2018

Description

This is an in-progress PR that adds a build-config package, which pulls the base webpack config into a package so it can be easily re-used by 3rd-party developers. It then adds a build.js script to @wordpress/scripts that leverages the config. Once merged, this would allow developers to either depend on the scripts package or run npx wp-scripts build to build blocks and extensions.

By having something like this in core, we'd have some simple build tools available out of the box so that block developers don't need to hunt down 3rd-party tools or configure webpack/babel manually. Further, the build-config package may also be useful for 3rd-party tool developers (e.g. create-guten-block, gutenblock, etc) who want to better ensure their compatibility with Gutenberg.

How has this been tested?

The first thing to test with this PR is just a normal Gutenberg build because much of the webpack config has been extracted into a package. From what I can tell, everything still builds correctly, tests pass, and Gutenberg appears to function as expected.

Testing block building using wp-scripts is a bit tricky because you're running scripts from unpublished packages only available locally. To make this work, I do the following:

  1. Build Gutenberg itself on this feature branch with npm run build.
  2. Clone the following example plugin into your plugins folder: https://github.com/griffbrad/wp-scripts-build-block-example-plugin
  3. In that plugin's folder, run npm run copy-local-gutenberg-packages && npm i.
  4. Run npm run build in the example plugin's folder. The block's code in src/index.js will be built with the output being placed in build.
  5. Activate the example plugin and ensure the "WP Scripts Build Tool" block is available and functioning.

These testing instructions copy the built Gutenberg plugin and its packages into the example plugin itself because I've found that using a file ref to traverse back to the parent directory causes NPM to randomly crash and spew non-sense.

Types of changes

New feature

Checklist:

  • My code is tested. (Working on a script that does some basic end-to-end testing of the process.)
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • Cleaner build script output. Currently just logging messages from webpack's compiler directly.
  • Simpler default for webpack output configuration.
  • Docs with examples for modifying the base webpack config when needed. Mostly thinking of this as a "zero configuration" approach, but I can foresee some common tweaks we may want to support.

Planned follow-up PRs

Assuming we get this first PR ready and merged, I'll likely add a separate dev script with a watch flag and an init script that sets up a basic template/boilerplate for at least block development.

@chrisvanpatten chrisvanpatten added the [Type] Build Tooling Issues or PRs related to build tooling label Dec 7, 2018
@gziolo gziolo added the [Tool] WP Scripts /packages/scripts label Dec 14, 2018
@gziolo gziolo self-requested a review December 14, 2018 13:24
@gziolo
Copy link
Member

gziolo commented Dec 14, 2018

Thanks for starting this PR. I missed it and started exploration for something similar but limited in scope in #12837. I will take a closer look at what you proposed next week 👍

@gziolo gziolo requested a review from ntwb December 14, 2018 13:27
Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

Nice idea to expose Webpack config. I think we should limit this PR to the config and tackle the script part separately.

package.json Show resolved Hide resolved
{
test: /\.js$/,
exclude: [
/block-serialization-spec-parser/,
Copy link
Member

Choose a reason for hiding this comment

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

We should do something to make it more bullet-proof. I have no idea how it will behave outside of Gutenberg repository. It was a quick workaround to make ES5 modules with Webpack.

packages/build-config/package.json Show resolved Hide resolved
packages/build-config/package.json Show resolved Hide resolved
packages/build-config/package.json Show resolved Hide resolved
const webpack = require( 'webpack' );
const formatWebpackMessages = require( 'react-dev-utils/formatWebpackMessages' );

webpackConfig.module.rules.forEach(
Copy link
Member

Choose a reason for hiding this comment

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

See my PR: #12837.

I think we should tackle @wordpress/scripts separately. It would be awesome to expose Webpack config first.

return `wp.${ name.replace( /-([a-z])/g, ( match, letter ) => letter.toUpperCase() ) }`;
};

const wordpressExternals = ( context, request, callback ) =>
Copy link
Member

Choose a reason for hiding this comment

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

Does it magically detect all imports starting with @wordpress/* and converts them into externals?

Would it make sense to apply all improvements to the config as its own PR first so we could test them in isolation?

@dmsnell
Copy link
Member

dmsnell commented Jan 3, 2019

so it can be easily re-used by 3rd-party developers

Do we have developers asking for this? I'm a little concerned that this is introducing more complexity into the build process and may not be used. It's taking our config one step further away from where people expect it.

Sorry - this isn't meant to sound critical - I'm just trying to understand the justification for this change.

@griffbrad
Copy link
Author

@dmsnell Basically, there is no official tooling or guidance regarding the JS build process for block and/or extension development currently. The Gutenberg Handbook offers ESNext examples but doesn’t offer a straightforward option for actually running that code. Extracing the webpack config allows us to leverage what is already in Gutenberg to offer something for those folks in the scripts package.
It could also improve compatability (for example, in the handling of RTL CSS) with blocks built by other tools (e.g. create-guten-block), if they could use the same webpack/babel config internally.

@gziolo
Copy link
Member

gziolo commented Jan 23, 2019

I have just landed #12837 which introduces wp-scripts start and wp-scripts run scripts to @wordpress/scripts package. Do you plan to update this PR to include the latest changes?

By the way, the topic of shared Webpack config was discussed yesterday during weekly Core JS chat (link requires registration):
https://wordpress.slack.com/archives/C5UNMSU4R/p1548165732173600.

@gziolo
Copy link
Member

gziolo commented Feb 12, 2019

I'm closing this in favor of #13814. I had to merge this code into Gutenberg repository so I could continue all the great work started here. Let's continue on the new PR 🙇

@gziolo gziolo closed this Feb 12, 2019
@gziolo gziolo deleted the try/reuse-webpack-config-for-build-script branch February 12, 2019 07:34
@gziolo gziolo removed [Status] In Progress Tracking issues with work in progress labels Jun 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Tool] WP Scripts /packages/scripts [Type] Build Tooling Issues or PRs related to build tooling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants