Build: Add building/watching support to Gutenberg packages #6708

youknowriad · 2018-05-11T13:23:40Z

This PR builds on top of #6658 and adds building support to the Gutenberg packages.

The build/watch scripts added in the PR are copied and adapter from Facebook Jest and WordPress packages scripts.

The idea is to consider these packages as real external modules. Building happens in two steps, the build script generates the build and build-module transpiled files and webpack uses these built files to generate its bundle with the library support (global variable).

This means for dev environments, we need to watch changes on each file individually, build this file which will then trigger the webpack watching to the compiled files.

herregroen · 2018-05-15T13:23:30Z

bin/packages/watch.js

+	}
+} );
+
+setInterval( () => {


Might be worth considering replacing this with a debounced function that's triggered in rebuild. Not really a major thing though.

herregroen · 2018-05-15T13:28:03Z

bin/packages/watch.js

+		fs.watch( path.resolve( p, 'src' ), { recursive: true }, ( event, filename ) => {
+			const filePath = path.resolve( srcDir, filename );
+
+			if ( ( event === 'change' || event === 'rename' ) && exists( filePath ) ) {


According to https://nodejs.org/docs/latest/api/fs.html#fs_fs_watch_filename_options_listener change and rename are the only two possible options for event this can probably be shortened to exists( filePath ).

gziolo · 2018-05-18T13:01:35Z

I added a few tweaks to this PR based on what I discovered while testing locally. It works great for me.

Some open questions:

Does this script included in this PR differs in any place from what we have in WordPress/packages?
Does postcss-themes should also have main and modules in package.json?

Tested so far:

npm i
npm test
npm run test-unit:coverage
npm run dev

Tested manually in the browser.

I also want to make some more in-depth analysis for bundles which we output after those changes got introduced.

gziolo · 2018-05-18T13:02:40Z

package.json

@@ -139,7 +139,7 @@
 		"fixtures:generate": "npm run fixtures:server-registered && cross-env GENERATE_MISSING_FIXTURES=y npm run test-unit",
 		"fixtures:regenerate": "npm run fixtures:clean && npm run fixtures:generate",
 		"package-plugin": "./bin/build-plugin-zip.sh",
-		"postinstall": "lerna bootstrap --hoist",
+		"postinstall": "lerna bootstrap --hoist && npm run build:packages",


We might need it later when we start referencing internal packages. This was the case for WordPress/packages at least.

gziolo · 2018-05-18T13:12:21Z

bin/packages/build.js

+/**
+ * Babel Configuration
+ */
+const babelDefaultConfig = require( '@wordpress/babel-preset-default' );


We should load setup from package.json - we use some overrides.

I think we should use a separate config here. The root config could contain things we don't want here.

For example: generating the "messages.pot" file should not happen here but should happen when webpack performs the building (it's not something we want to include per module).

Yes, this is also what I figured out when trying to update this config yesterday.

I published new version of Babel preset to include missing async generator functions plugin.

I'm wondering something though about the "messages.pot" generation :). babel may change variable names and function names and this could cause the make-pot to fail and not find the right strings. So in the end it might be better to include it here?

#6893 will cover the support for async generator functions.

gziolo · 2018-05-22T11:40:14Z

Comparison of Webpack generated assets

Before

                                  Asset       Size          Chunks             Chunk Names
                  ./build/a11y/index.js   9.32 KiB            a11y  [emitted]  a11y
            ./build/components/index.js   1.58 MiB      components  [emitted]  components
           ./build/core-blocks/index.js    583 KiB      coreBlocks  [emitted]  coreBlocks
             ./build/core-data/index.js    187 KiB        coreData  [emitted]  coreData
                  ./build/data/index.js    263 KiB            data  [emitted]  data
                  ./build/date/index.js    212 KiB            date  [emitted]  date
                   ./build/dom/index.js   79.8 KiB             dom  [emitted]  dom
             ./build/dom-ready/index.js   3.56 KiB        domReady  [emitted]  domReady
             ./build/edit-post/index.js    346 KiB        editPost  [emitted]  editPost
                ./build/editor/index.js   1.45 MiB          editor  [emitted]  editor
               ./build/element/index.js    158 KiB         element  [emitted]  element
                 ./build/hooks/index.js   56.2 KiB           hooks  [emitted]  hooks
                  ./build/i18n/index.js   78.2 KiB            i18n  [emitted]  i18n
      ./build/is-shallow-equal/index.js    5.8 KiB  isShallowEqual  [emitted]  isShallowEqual
               ./build/plugins/index.js    112 KiB         plugins  [emitted]  plugins
                 ./build/utils/index.js    123 KiB           utils  [emitted]  utils
              ./build/viewport/index.js   64.9 KiB        viewport  [emitted]  viewport
                ./build/blocks/index.js    778 KiB          blocks  [emitted]  blocks

After

                                   Asset       Size          Chunks             Chunk Names
                   ./build/a11y/index.js   9.32 KiB            a11y  [emitted]  a11y
             ./build/components/index.js   1.58 MiB      components  [emitted]  components
            ./build/core-blocks/index.js    583 KiB      coreBlocks  [emitted]  coreBlocks
              ./build/core-data/index.js    187 KiB        coreData  [emitted]  coreData
                   ./build/data/index.js    263 KiB            data  [emitted]  data
                   ./build/date/index.js    213 KiB            date  [emitted]  date
                    ./build/dom/index.js   79.9 KiB             dom  [emitted]  dom
              ./build/dom-ready/index.js   3.56 KiB        domReady  [emitted]  domReady
              ./build/edit-post/index.js    346 KiB        editPost  [emitted]  editPost
                 ./build/editor/index.js   1.45 MiB          editor  [emitted]  editor
                ./build/element/index.js    158 KiB         element  [emitted]  element
                  ./build/hooks/index.js   56.2 KiB           hooks  [emitted]  hooks
                   ./build/i18n/index.js   78.2 KiB            i18n  [emitted]  i18n
       ./build/is-shallow-equal/index.js    5.8 KiB  isShallowEqual  [emitted]  isShallowEqual
                ./build/plugins/index.js    112 KiB         plugins  [emitted]  plugins
                  ./build/utils/index.js    123 KiB           utils  [emitted]  utils
               ./build/viewport/index.js   64.9 KiB        viewport  [emitted]  viewport
                 ./build/blocks/index.js    778 KiB          blocks  [emitted]  blocks

gziolo

I didn't discover any regressions. Build files generated by Webpack look almost identical to what we had before. Let's proceed and see how it works.

I think we will have to ask people to run npm i on their repositories.

youknowriad · 2018-05-22T12:34:45Z

I'm personally still concerned about the messages extraction. What will happen when some messages are in packages and others are still in the current modules? how do we make the extraction work? cc @aduth

aduth · 2018-05-24T20:09:14Z

I've not really kept up with how the packages integration has progressed. I'm also not sure whether we'd want packages themselves to provide any pre-extracted strings data, or if we just assume they call __, and if something else happens to provide the localized data (through @wordpress/i18n's setLocaleData) it will be used. Ultimately we'd want some form of per-script-handle string localization, which is still in early stages (#6015, #6051, "JavaScript Internationalization: The Missing Pieces").

As for where this stands currently, have you tried generating the POT file to see if it includes neither/either/both of packages and non-packages strings? Since our Webpack's babel-loader is configured to exclude only node_modules and Gutenberg packages are directed to ./packages, I expect it should be able to pick up on all of these strings.

youknowriad · 2018-05-25T12:37:31Z

Ok did some tests and it should work because in the build-module scripts the imports are not updated. Let's get this in and see where it brings us.

youknowriad requested review from aduth, gziolo, pento and ntwb and removed request for aduth May 11, 2018 13:24

youknowriad self-assigned this May 11, 2018

youknowriad added the [Type] Build Tooling Issues or PRs related to build tooling label May 11, 2018

herregroen reviewed May 15, 2018

View reviewed changes

youknowriad and others added 3 commits May 18, 2018 14:35

Build: Add building/watching support to Gutenberg packages

9657d4c

Packages: Add module entry points for packages

43b183a

Packages: Improve configuration for builds

751fdb3

gziolo force-pushed the add/building-support branch from 7faa1be to 751fdb3 Compare May 18, 2018 12:54

gziolo reviewed May 18, 2018

View reviewed changes

gziolo mentioned this pull request May 21, 2018

Dashicons use wp.element.Component instead of importing it #3952

Closed

gziolo added this to the 3.0 milestone May 21, 2018

gziolo mentioned this pull request May 22, 2018

Packages: Update all WordPress packages to the latest version #6893

Merged

4 tasks

gziolo approved these changes May 22, 2018

View reviewed changes

youknowriad merged commit a5aad35 into master May 25, 2018

youknowriad deleted the add/building-support branch May 25, 2018 12:37

aduth mentioned this pull request Apr 26, 2019

Build Tooling: Pass individual files as arguments from watch to build script #15219

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Build: Add building/watching support to Gutenberg packages #6708

Build: Add building/watching support to Gutenberg packages #6708

youknowriad commented May 11, 2018

herregroen May 15, 2018

herregroen May 15, 2018

gziolo commented May 18, 2018

gziolo May 18, 2018

gziolo May 18, 2018

youknowriad May 21, 2018

gziolo May 22, 2018

youknowriad May 22, 2018 •

edited

Loading

gziolo May 22, 2018

gziolo commented May 22, 2018 •

edited

Loading

gziolo left a comment

youknowriad commented May 22, 2018

aduth commented May 24, 2018

youknowriad commented May 25, 2018

Build: Add building/watching support to Gutenberg packages #6708

Build: Add building/watching support to Gutenberg packages #6708

Conversation

youknowriad commented May 11, 2018

herregroen May 15, 2018

Choose a reason for hiding this comment

herregroen May 15, 2018

Choose a reason for hiding this comment

gziolo commented May 18, 2018

gziolo May 18, 2018

Choose a reason for hiding this comment

gziolo May 18, 2018

Choose a reason for hiding this comment

youknowriad May 21, 2018

Choose a reason for hiding this comment

gziolo May 22, 2018

Choose a reason for hiding this comment

youknowriad May 22, 2018 • edited Loading

Choose a reason for hiding this comment

gziolo May 22, 2018

Choose a reason for hiding this comment

gziolo commented May 22, 2018 • edited Loading

Comparison of Webpack generated assets

Before

After

gziolo left a comment

Choose a reason for hiding this comment

youknowriad commented May 22, 2018

aduth commented May 24, 2018

youknowriad commented May 25, 2018

youknowriad May 22, 2018 •

edited

Loading

gziolo commented May 22, 2018 •

edited

Loading