Read script dependencies from generated files #15124

sirreal · 2019-04-23T14:05:43Z

Remove manually maintained static list of script dependencies.
Read script dependencies from dependency files generated by build tools.
Replace translation override usage of static list of scripts.
Enable injectPolyfill option for the webpack plugin. Consumers of wp-scripts default webpack build will also receive this behavior.
Remove addition of wp-polyfill dependency on script registration. It's handled by the webpack plugin.
Include build/[package]/*.deps.json dependency files in plugin zip
Handle undetected dependencies: editor, media-models, media-views, postbox.

Description

Use generated script dependencies

How has this been tested?

Prepare local testing environment and npm run build. Does the block editor work as expected?

Try building a plugin zip (./bin/build-plugin-zip.sh) then installing and activating it on another site. Does it work as expected?
Sample from build plugin zip:

adding: build/a11y/index.deps.json (stored 0%)
adding: build/a11y/index.js (deflated 54%)
adding: build/annotations/index.deps.json (deflated 20%)
adding: build/annotations/index.js (deflated 67%)
adding: build/api-fetch/index.deps.json (deflated 15%)
adding: build/api-fetch/index.js (deflated 62%)
adding: build/autop/index.deps.json (stored 0%)
adding: build/autop/index.js (deflated 64%)

Types of changes

Improve build tooling.

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
My code has proper inline documentation.
I've included developer documentation if appropriate.

gziolo · 2019-04-23T14:46:03Z

We will need to expose *.deps.json files in the zip file for the plugin's package:
https://github.com/WordPress/gutenberg/blob/master/bin/build-plugin-zip.sh#L110

lib/packages-dependencies.php

sirreal · 2019-04-24T10:09:49Z

packages/scripts/config/webpack.config.js

@@ -67,7 +67,7 @@ const config = {
 		// WP_LIVE_RELOAD_PORT global variable changes port on which live reload works
 		// when running watch mode.
 		! isProduction && new LiveReloadPlugin( { port: process.env.WP_LIVE_RELOAD_PORT || 35729 } ),
-		new DependencyExtractionWebpackPlugin(),
+		new DependencyExtractionWebpackPlugin( { injectPolyfill: true } ),


Includes wp-polyfill as a script dependency for every entrypoint.

Includes wp-polyfill as a script dependency for every entrypoint.

Looking at the implementation, would wp-polyfill include wp-polyfill as a dependency of itself?

It would yes.

That said, I don't see polyfill being built as part of Gutenberg (no build/polyfill/index.deps.json). Is this a theoretical concern or is the polyfill built somewhere we'd need to be careful with this?

I don't think we will ever scan wp-polyfill but we can add guard clause to be on the safe side.

sirreal · 2019-04-24T10:11:14Z

lib/client-assets.php

+
+	// Ignore scripts that are not found in the expected `build/` location.
+	$script_path = gutenberg_dir_path() . 'build/' . substr( $handle, 3 ) . '/index.js';
+	if ( ! file_exists( $script_path ) ) {


I'm unsure how to test whether this is working well, I'd appreciate some help.

That's a great question. Travis seems to be happy about changes applied 🤷‍♂️

How about we compare all script tags in the HTML output between master and this branch?

This is rather tricky to verify, but it seems to be working. Here's what I did:

Build the plugin zip from this PR.

Get a new WordPress site.

Install the plugin from the zip.

Change the site language.

Go to updates and install language updates. This will download the missing language files for the Gutenberg plugin we just uploaded.

Open the editor.

Check a known string: wp.i18n.__( 'Use the classic WordPress editor.' )

Find the plugin language file containing your string. In my case /wp-content/languages/plugins/gutenberg-es_ES-d83116db7e3459d9c9a90cae3216278f.json

Edit the language file changing the translation of your string (Utiliza el editor clásico de WordPress.) for something else (👋 Hi from the plugin language file.)

Activate the plugin.

Check the string from the editor again: wp.i18n.__( 'Use the classic WordPress editor.' )

Should be your new string.

Deactivate the plugin.

Back to the original string.

sirreal · 2019-04-24T12:06:24Z

Some tests are failing on CI, I'll have a look…

gziolo · 2019-04-24T12:16:49Z

Some tests are failing on CI, I'll have a look…

Skip them, those are tests which fail also for other PRs. I'm going to skip those tests later this week to avoid false negatives ...

gziolo · 2019-04-24T12:18:25Z

lib/client-assets.php

+			: array();
+
+		// Add dependencies that cannot be detected and generated by build tools.
+		switch ( $handle ) {


Works for me 🤷‍♂️

😅

gziolo · 2019-04-25T13:36:36Z

@omarreiss @atimmer - this is something we can add to the workflow in WordPress core as well. We probably will have to look at extending metadata included in this JSON file with the hash generated based on the content of build file to ensure it gets cached in the browser for as long as it doesn't change.

Some scripts have dependencies that are undetectable by the webpack plugin used to generate the dependency files. Add these dependencies to the generated dependencies.

Enable `injectPolyfill` option in the webpack plugin instead of manually injecting the dependency via PHP on script registration. This will enable the polyfill for consumers of wp-scripts default webpack config.

gziolo

It looks like this PR is ready to go. I tested it locally and it works as expected. Great work @sirreal!!!

This work to automate dependencies management will greatly improve the experience moving forward 💯

aduth · 2019-04-30T13:11:03Z

lib/client-assets.php

@@ -177,17 +182,37 @@ function gutenberg_override_style( $handle, $src, $deps = array(), $ver = false,
 * @since 4.5.0
 */
 function gutenberg_register_packages_scripts() {
-	$packages_dependencies = include dirname( __FILE__ ) . '/packages-dependencies.php';
+	foreach ( glob( gutenberg_dir_path() . 'build/*/index.js' ) as $path ) {


Depending if and how similar code would make its way to core, I'm a bit sensitive to the performance implications of a glob.

https://gschoppe.com/wordpress/benchmarking-gutenberg/#comment-3154

https://www.phparch.com/2010/04/putting-glob-to-the-test/

Seems like there could be optimizations:

Avoiding the folder scan and pregenerating the scripts at build time (similar to generate-gutenberg-php.php)

Substituting glob with something faster (opendir)

👍A nice enhancement for a future version or simply another part of the build would be to produce what is needed in PHP without needing to hit the filesystem or parse JSON.

This came up during the core-js slack chat.

CC: @gziolo

Related conversation on WordPress Slack (link requires registration):

https://wordpress.slack.com/archives/C5UNMSU4R/p1556633084220800

We didn't discuss file discovery which would be still an issue.

commit 1dddab4 Author: Stefanos Togkoulidis <[email protected]> Date: Wed May 1 00:00:50 2019 +0300 Have Aztec delete the detected Enter key for paragraphs Aztec-Android doesn't swallow the Enter key (like the list handling does) so, instruct Aztec to delete it for the paragraph block. commit 0936ca0 Author: Stefanos Togkoulidis <[email protected]> Date: Tue Apr 30 20:58:36 2019 +0300 Just use onFormatChange which now defaults to "force" commit 6358de3 Merge: a5282ce 5e4d627 Author: Stefanos Togkoulidis <[email protected]> Date: Tue Apr 30 20:23:57 2019 +0300 Merge branch 'master' into rnmobile/fix-list-handling-android commit 5e4d627 Author: Pascal Birchler <[email protected]> Date: Tue Apr 30 18:27:24 2019 +0200 Update to Babel 7.4 and core-js 3 (#15139) * Specify core-js version when using useBuiltIns * Update to Babel 7.4 and core-js 3 * setupFiles: fix path to async-iterator * Also update Babel packages in babel-preset-default * Update snapshot for babel-preset-default test * Manually include web.dom-collections.iterator in jest config * Regenerate package-lock.json file after latest changes applied * Upgrade ESLint related npm packages * Another cleanup in the package-lock.json file * Remove ESLint rules which throw error to fix linting * Add core-js/modules/web.dom.iterable to make e2e tests work * Update Babel packages to the latest version * Update ignore files in e2e tests config * Add changelog entries related to Babel and ESLint version bumps commit 1d959ba Author: Andrés <[email protected]> Date: Tue Apr 30 17:14:35 2019 +0200 Remove not used state (#15224) commit d715e93 Author: Jon Surrell <[email protected]> Date: Tue Apr 30 16:29:09 2019 +0200 Babel plugin JSX: Implement Fragment handling (#15120) Add imports for `<></>` JSX Fragments. commit 96cad99 Author: Kelly Dwan <[email protected]> Date: Tue Apr 30 09:18:54 2019 -0400 Fix incorrect ID in FocalPointPicker (#15255) commit c1ba13e Author: Jon Surrell <[email protected]> Date: Tue Apr 30 13:44:50 2019 +0200 Update Fragment imports in READMEs (#15262) Some Fragment imports were incorrect. Fix them. commit 297c2f4 Author: etoledom <[email protected]> Date: Tue Apr 30 13:19:34 2019 +0200 RNMobile: Fix autoscroll on ListBlock (#15048) * RNMobile: Add ability to send extra props (i.e. props needed just for mobile) to the list block. * Adding onCaretVerticalPositionChange to RichText via context * Passing onCaretVerticalPositionChange via context to RichText for all RichText based blocks This makes it not necessary to pass onCaretVerticalPositionChange as a prop directly to RichText from the block component. * Update jest snapshot for block-edit commit 447dd27 Author: Jorge Costa <[email protected]> Date: Tue Apr 30 10:47:55 2019 +0100 Fix copy paste and delete error core paragraph with locking (#14712) * Fix: Copy / Paste error core/paragraph with locking * Add test case. * Update packages/block-library/src/paragraph/edit.js Co-Authored-By: jorgefilipecosta <[email protected]> commit 0b2eb1c Author: Jon Surrell <[email protected]> Date: Tue Apr 30 09:39:29 2019 +0200 Dependency extraction webpack plugin: Change option name consistent with docs (#15260) In the docs, an option name was provided as `requestToHandle`, but the actual option in use was `requestToDependency`. Update the code and related tests to use `requestToHandle` as described in the documentation instead of `requestToDependency`. commit 506827e Author: Jon Surrell <[email protected]> Date: Tue Apr 30 09:31:38 2019 +0200 Read script dependencies from generated files (#15124) * Rework script registration to use generated deps * Deprecate static list of dependencies * Rework translation without packages-dependencies.php * Include build/*/*.deps.json in plugin zip * Add script dependencies undetectable by build tools Some scripts have dependencies that are undetectable by the webpack plugin used to generate the dependency files. Add these dependencies to the generated dependencies. * Add `wp-polyfill` dependency via webpack plugin Enable `injectPolyfill` option in the webpack plugin instead of manually injecting the dependency via PHP on script registration. This will enable the polyfill for consumers of wp-scripts default webpack config. * Improve deprecated message * Remove obsolete lib/packages-dependencies.php commit c2c8276 Author: Daniel Richards <[email protected]> Date: Tue Apr 30 13:36:49 2019 +0800 Use button block appender on group block (#14943) * Use button block appender on group block * Update group block e2e tests for the new button appender * Add additional margin to the appender when a group has a background color. * Use more opaque versions of the appender background color To ensure greater compatibility when layered on top of a block with a background color. * Block Library: Display ButtonBlockAppender only if selected or empty * Only render button appender when the group has no inner blocks commit 1e96ec7 Author: Andrew Duthie <[email protected]> Date: Mon Apr 29 16:40:24 2019 -0400 Build Tooling: Pass individual files as arguments from watch to build script (#15219) commit cafb57b Author: Grzegorz (Greg) Ziółkowski <[email protected]> Date: Mon Apr 29 22:12:08 2019 +0200 Blocks: Upgrade simple-html-tokenizer dependency (#15246) commit 5b3b3ab Author: Andrew Duthie <[email protected]> Date: Mon Apr 29 13:38:09 2019 -0400 Framework: Add REST API codeowners (#15215) commit 64c48fd Author: Ian Dunn <[email protected]> Date: Mon Apr 29 10:06:46 2019 -0700 Correct name of `date_i18n()` PHP function. (#15204) commit 8c28814 Author: Andrew Duthie <[email protected]> Date: Mon Apr 29 13:05:10 2019 -0400 Framework: Remove TESTS.md (#15217) commit 0f1fb57 Author: Emmanuel Hesry <[email protected]> Date: Mon Apr 29 17:34:22 2019 +0200 fix typo in withDispatch documentation (#15251) commit a5282ce Author: Stefanos Togkoulidis <[email protected]> Date: Thu Apr 25 16:08:31 2019 +0300 Revert "Trivial change to trigger Travis" This reverts commit e22ffde. commit e22ffde Author: Stefanos Togkoulidis <[email protected]> Date: Thu Apr 25 16:08:24 2019 +0300 Trivial change to trigger Travis commit 15a51f8 Author: Jorge Costa <[email protected]> Date: Fri Apr 19 15:33:15 2019 +0100 chore: Fix: Lint error that makes unit tests (and CI tests) fail. (#15073) commit 2b11074 Author: Stefanos Togkoulidis <[email protected]> Date: Thu Apr 25 12:42:55 2019 +0300 Fix lint issues commit 9c298c8 Author: Stefanos Togkoulidis <[email protected]> Date: Thu Apr 25 12:01:46 2019 +0300 Need to specify firedAfterTextChanged on all Aztec events commit 46e086d Author: Stefanos Togkoulidis <[email protected]> Date: Thu Apr 25 03:59:59 2019 +0300 Force Aztec update if "Enter" fired before text change commit 724e295 Author: Stefanos Togkoulidis <[email protected]> Date: Thu Apr 25 03:11:52 2019 +0300 Differentiate Android and iOS since assumptions diverged The iOS side still expects to just check against `this.lastContent` to force the change into Aztec. commit 52386fa Author: Stefanos Togkoulidis <[email protected]> Date: Thu Apr 25 01:55:22 2019 +0300 Use a flag to signal Aztec-originated changes And assume that when that flag is false, component changes need to get sent/reflected down to Aztec. commit c7aa381 Author: Stefanos Togkoulidis <[email protected]> Date: Tue Apr 23 17:30:45 2019 +0300 Able to not lose content commit 60c75b0 Author: Stefanos Togkoulidis <[email protected]> Date: Tue Apr 23 10:09:23 2019 +0300 If text already changed, don't modify it

sirreal added [Status] In Progress Tracking issues with work in progress [Type] Build Tooling Issues or PRs related to build tooling labels Apr 23, 2019

sirreal self-assigned this Apr 23, 2019

sirreal changed the title ~~Update/14837 read generated script dependencies~~ Read script dependencies from generated files Apr 23, 2019

sirreal force-pushed the update/14837-read-generated-script-dependencies branch 3 times, most recently from fe5599d to 9a9cba1 Compare April 24, 2019 09:43

sirreal commented Apr 24, 2019

View reviewed changes

sirreal added [Type] Enhancement A suggestion for improvement. Needs Technical Feedback Needs testing from a developer perspective. and removed [Status] In Progress Tracking issues with work in progress labels Apr 24, 2019

sirreal marked this pull request as ready for review April 24, 2019 10:17

sirreal requested review from aduth, ajitbohra, gziolo, mkaz, nerrad, oandregal, ntwb and youknowriad as code owners April 24, 2019 10:17

sirreal added the [Status] In Progress Tracking issues with work in progress label Apr 24, 2019

gziolo reviewed Apr 24, 2019

View reviewed changes

sirreal force-pushed the update/14837-read-generated-script-dependencies branch from d26c1fb to 5b765aa Compare April 24, 2019 13:10

gziolo requested review from omarreiss and atimmer April 25, 2019 13:34

Rework script registration to use generated deps

906250e

sirreal added 7 commits April 30, 2019 09:06

Rework translation without packages-dependencies.php

993c8a8

Deprecate static list of dependencies

f540605

Include build/*/*.deps.json in plugin zip

33991d6

Add script dependencies undetectable by build tools

2c3b654

Some scripts have dependencies that are undetectable by the webpack plugin used to generate the dependency files. Add these dependencies to the generated dependencies.

Add wp-polyfill dependency via webpack plugin

27bccbe

Enable `injectPolyfill` option in the webpack plugin instead of manually injecting the dependency via PHP on script registration. This will enable the polyfill for consumers of wp-scripts default webpack config.

Improve deprecated message

df21c64

Remove obsolete lib/packages-dependencies.php

6de12cc

gziolo force-pushed the update/14837-read-generated-script-dependencies branch from 5b765aa to 6de12cc Compare April 30, 2019 07:06

gziolo removed [Status] In Progress Tracking issues with work in progress Needs Technical Feedback Needs testing from a developer perspective. labels Apr 30, 2019

gziolo approved these changes Apr 30, 2019

View reviewed changes

gziolo merged commit 506827e into master Apr 30, 2019

gziolo deleted the update/14837-read-generated-script-dependencies branch April 30, 2019 07:31

gziolo mentioned this pull request Apr 30, 2019

Plugin: Seek to eliminate or substitute packages for globals references #15125

Open

aduth reviewed Apr 30, 2019

View reviewed changes

youknowriad added this to the 5.7 (Gutenberg) milestone May 10, 2019

sirreal mentioned this pull request Sep 2, 2019

Build Tooling: Generate asset file in PHP format with the list dependencies #17298

Merged

5 tasks

jrchamp mentioned this pull request Sep 12, 2019

Documentation: Add tutorial explaining how to use the autogenerated asset file when using wp-scripts #17423

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Read script dependencies from generated files #15124

Read script dependencies from generated files #15124

sirreal commented Apr 23, 2019 •

edited

Loading

gziolo commented Apr 23, 2019

sirreal Apr 24, 2019

aduth Apr 30, 2019

sirreal Apr 30, 2019

gziolo Apr 30, 2019 •

edited

Loading

sirreal Apr 24, 2019

gziolo Apr 24, 2019

sirreal Apr 24, 2019

sirreal commented Apr 24, 2019

gziolo commented Apr 24, 2019

gziolo Apr 24, 2019

gziolo commented Apr 25, 2019

gziolo left a comment

aduth Apr 30, 2019

sirreal Apr 30, 2019

gziolo Apr 30, 2019 •

edited

Loading

Read script dependencies from generated files #15124

Read script dependencies from generated files #15124

Conversation

sirreal commented Apr 23, 2019 • edited Loading

Description

How has this been tested?

Types of changes

Checklist:

gziolo commented Apr 23, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gziolo Apr 30, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sirreal commented Apr 24, 2019

gziolo commented Apr 24, 2019

Choose a reason for hiding this comment

gziolo commented Apr 25, 2019

gziolo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gziolo Apr 30, 2019 • edited Loading

Choose a reason for hiding this comment

sirreal commented Apr 23, 2019 •

edited

Loading

gziolo Apr 30, 2019 •

edited

Loading

gziolo Apr 30, 2019 •

edited

Loading