WordPress.org

Make WordPress Core

Opened 14 months ago

Closed 12 months ago

Last modified 12 months ago

#45065 closed enhancement (fixed)

Include Gutenberg packages in WordPress core.

Reported by: omarreiss Owned by: omarreiss
Milestone: 5.0 Priority: normal
Severity: normal Version:
Component: General Keywords: fixed-5.0
Focuses: javascript Cc:
PR Number:

Description

The Gutenberg JavaScript packages will continue to be maintained on Github and distributed through NPM.

To include the Gutenberg packages in WordPress we'll need to:

  • add them as dependencies in WordPress' package.json
  • build all the necessary packages using Webpack
  • register the build files so they can be enqueued. This unblocks moving the PHP over to WordPress core.

Attachments (10)

45065-wip.diff (846.1 KB) - added by atimmer 14 months ago.
45065-wip.2.diff (116.8 KB) - added by atimmer 14 months ago.
45065-wip.3.diff (119.9 KB) - added by atimmer 14 months ago.
45065-wip.4.diff (119.9 KB) - added by atimmer 14 months ago.
45065.diff (173.5 KB) - added by atimmer 14 months ago.
45065.2.diff (225.2 KB) - added by atimmer 14 months ago.
45065.3.diff (226.6 KB) - added by atimmer 14 months ago.
45065.4.diff (225.2 KB) - added by atimmer 14 months ago.
45065.5.diff (225.1 KB) - added by atimmer 14 months ago.
45065-script-loader.diff (16.5 KB) - added by atimmer 14 months ago.

Change History (51)

#1 @atimmer
14 months ago

In 43687:

Build tools: Combine webpack config files.

This prepares us for building the Gutenberg packages.

See #45065.

#2 @pento
14 months ago

  • Milestone changed from Awaiting Review to 5.0

#3 @atimmer
14 months ago

In 43688:

Build tools: Upgrade webpack to version 4.

  • Minification is done by uglify, so disable that in the media build.
  • The webpack boilerplate has changed, which explains the changes in the build files.
  • ModuleConcatenationPlugin is enable by default for production builds so we don't have to specify that ourselves.

See #45065.

This ticket was mentioned in Slack in #core-js by atimmer. View the logs.


14 months ago

@atimmer
14 months ago

#5 @atimmer
14 months ago

45065-wip.diff is a working patch that builds all available necessary Gutenberg packages from npm.

#6 @atimmer
14 months ago

45065-wip.2.diff doesn't include the media files with inline sourcemaps.

This ticket was mentioned in Slack in #core-js by netweb. View the logs.


14 months ago

@atimmer
14 months ago

#8 @atimmer
14 months ago

45065.diff is the latest status. I've changed the vendor handling to copy from node_modules instead of building them with a custom Webpack config.

After the missing packages have been published to npm I can commit this and continue work with the PHP registering of the assets.

PS. The discussion about this patch is located on GitHub: https://github.com/Yoast/wordpress-develop-mirror/pull/118. That makes it easier to comment on lines in the patch.

@atimmer
14 months ago

@atimmer
14 months ago

@atimmer
14 months ago

@atimmer
14 months ago

#9 @atimmer
14 months ago

In 43719:

Build tools: Build @wordpress packages with webpack.

We decided to split the media webpack config into it's own file. The
main webpack config then combines this file with the packages config.

Include vendor scripts by copying them. We copy the minified files if
they are available. If they aren't available we minify the original
files ourselves.

Props omarreiss, herregroen, gziolo, youknowriad, netweb, adamsilverstein.
See #45065.

#11 @SergeyBiryukov
14 months ago

Travis failures were caused by using an outdated Node version, fixed in [43721].

#13 @atimmer
14 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 43723:

Script loader: Register @wordpress scripts.

This allows the packages to be consumed by plugins and core itself.
The code has been based on the work done in the Gutenberg plugin.

We've added an array with all the packages and the vendor packages to
loop through. This sets a convention so all packages will be
registered in the same way. This array can eventually be generated by
a webpack plugin.

We need to register TinyMCE explicitly. Previously TinyMCE was used
by inserting custom <script> tags into the relevant admin pages.
This is not suitable for the new editor, so we need to explicitly
register TinyMCE. We could, in the future, refactor the custom
<script> tags to use the registered TinyMCE script instead.

Polyfills are inserted into the page only when necessary using
document.write.

Props omarreiss, herregroen, youknowriad, gziolo.
Fixes #45065.

#14 @youknowriad
14 months ago

#44987 was marked as a duplicate.

This ticket was mentioned in Slack in #core-privacy by azaozz. View the logs.


14 months ago

This ticket was mentioned in Slack in #core-committers by pento. View the logs.


14 months ago

#17 @pento
14 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Thank you for getting this done so quickly, @atimmer!

There are a few changes that we need to make here. Nothing major, just a bit of shuffling. 🙂

Instead of calling wp_default_packages_vendor(), wp_default_packages_scripts(), and wp_default_packages_inline_scripts() inside wp_default_scripts(), they should be attached to the wp_default_scripts action. wp_register_tinymce_scripts() should be attached to that action, too.

Is there any particular reason for wp_default_packages_vendor() and wp_default_packages_scripts() to be different functions? We don't differentiate between third party and WordPress scripts in wp_default_scripts(). They could be combined into a new wp_default_packages() function.

Similarly, wp_register_tinymce_scripts() could either be merged wp_default_packages().

wp_default_packages_inline_scripts() should be merged into wp_default_packages(), in the same style as inline scripts are added in wp_default_scripts().

There are several filters being run in wp_default_packages_inline_scripts() that don't have docblocks.

wp_register_tinymce_scripts() is a partial duplication of _WP_Editors::print_tinymce_scripts(): these need to be combined as much as possible, I suspect wp_register_tinymce_scripts() could really be merged into wp_default_packages() or wp_default_scripts(), too.

wp-tinymce-lists is trying to load index.js, it should be plugin.js.

All of the URL strings should be in the form "foo/bar/baz$suffix.js", rather than 'foo/bar/baz' . $suffix . '.js' or "foo/bar/baz{$suffix}.js".

I get where you were going with wp_default_packages_scripts() and wp_default_packages_vendor() being arrays of scripts that you then loop over to register, but this is probably going to cause maintenance headaches down the road. You can see in wp_default_scripts() how there are weird little variations in how scripts are loaded, a future version of WordPress will likely need to break these scripts out of the array, so we can do the same thing. Let's just do it the same way as wp_default_scripts().

Let's leave $script being accepted as a reference for now: it will probably go away as part of #44979, but that issue needs further investigation.

#18 @pento
14 months ago

[43688] added mode to the media build. This means that grunt webpack:dev causes all of the media-*.js files to be built in development mode, which is going to cause some huge patches to be accidentally submitted.

Let's remove that for now, we can really only make use of it when the full JS reorg is in place.

#19 @atimmer
14 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 43738:

Script loader: Adjust JS packages registration.

Adjusts the packages registration after [43723]:

  • Combine the different registration functions into one

wp_default_packages function. To reach this goal move the prefix
logic into a function so it can be called from different locations.
Use a static variable there to prevent duplicate inclusion of
version.php.

  • Call this function from the wp_default_scripts action by

registering it as a default filter.

  • Combine some of the logic in _WP_Editors::print_tinymce_scripts

into wp_register_tinymce_scripts. The logic to force an uncompressed
TinyMCE script file stays in _WP_Editors::force_uncompressed_tinymce
because that logic is very specific to the classic editor.

  • The script handle wp-tinymce is now a dependency of the editor

script handle. In combination with the previous item, this makes the
classic editor work.

  • Adjust the syntax of the script paths to be more consistent with

other WordPress code.

  • Always use "production" mode for the media files to prevent people

from inadvertently committing development files.

Props pento, omarreiss.
Fixes #45065.

#20 @atimmer
14 months ago

@pento I've processed your feedback in [43738].

I merged wp_default_packages_vendor, wp_register_tinymce_scripts, wp_default_packages_scripts and wp_default_packages_inline_scripts into wp_default_packages but kept the functionality in separate functions to keep a decent separation of concerns.

I chose to leave the array for now. I think it's actually a good thing that it discourages custom tweaks and invites better solutions. If tweaks really turn out to be necessary, we can always refactor.

If there's anything else to address I can commit that tomorrow.

#21 @azaozz
14 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

[43738] introduces a possible regression. It changes the order of loading of tinymce.js in class-wp-editor.php (not related to script-loader or Gutenberg).

Before the changes do_action( 'before_wp_tiny_mce', self::$mce_settings ); was firing before tinymce.js was loaded. Now it fires after.

Similarly tinyMCEPreInit was defined before tinymce.js was loaded, now it's defined after. This is especially important as TinyMCE looks for tinyMCEPreInit to get some predefined settings, see https://github.com/tinymce/tinymce/blob/7dfd8102cadfb35e7304cacec7f122aa9e81b1ad/src/core/main/ts/api/EditorManager.ts#L195.

Both happen as tinymce.js is now always enqueued through script-loader, regardless of where it is used.

Both of these don't affect loading of TinyMCE in core, but may affect plugins. Since we are trying to be very careful not to break anything in the 5.0 branch, thinking that change should be reverted.

If we need to abstract some parts of class-wp-editor.php, lets do it, but thinking we shouldn't change any of the functionality there, at least not in 5.0 :)

Last edited 14 months ago by azaozz (previous) (diff)

#22 @atimmer
14 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 43753:

Script loading: Fix regression after [43738].

After [43738], TinyMCE would be loaded earlier than before, which
makes filters run at a different time relative to the loading of
TinyMCE. Fix this by calling wp_print_scripts at the location where
TinyMCE would previously be inserted as a <script> tag in the page.

Props azaozz, omarreiss.
Fixes #45065.

#23 @atimmer
14 months ago

@azaozz Thank you for noticing the regression. I've fixed it in [43753].

#24 @atimmer
14 months ago

In 43754:

Script loading: Fix a PHP error introduced in [43753].

See #45065.
Props swissspidy.

#25 @youknowriad
14 months ago

@atimmer Will this change cause wp-tinymce to not load in Gutenberg? (I mean it will because it's defined as a dependency elsewhere too but it feels like it's not an ideal fix).

Theory:

Imagine Gutenberg switches TinyMCE over something else in the other scripts while still loading the editor script for the classicBlock or something, it will break right?

I thought I'd raise but I understand that it might not be so simple and that right now, Gutenberg do depend on tinymce elsewhere.

#26 @SergeyBiryukov
14 months ago

  • Keywords fixed-5.0 added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for merging to trunk.

#27 @atimmer
14 months ago

@youknowriad The current state on 5.0 is compatible with the current state of 4.9.8. Which means that I don't expect it to break. Gutenberg calls the relevant _WP_Editors methods itself, so even if Gutenberg were to remove TinyMCE, the <script> tag would still be printed by wp_print_scripts in print_tinymce_scripts.

#28 @pento
14 months ago

In 43802:

Script Loader: Fix metadata being registered as an inline script.

[43723] included script metadata for the wp-polyfill script that was being registered as an inline script.

See #45065.

#29 @pento
14 months ago

In 43803:

Scripts: wp-url is a dependency of wp-api-fetch.

See #45065.

#30 @pento
14 months ago

In 43805:

REST API: Add endpoints for blocks.

WP_REST_Block_Renderer_Controller allows rendering of server-side rendered blocks, whilst WP_REST_Blocks_Controller allows retrieving of reusable blocks.

Props desrosj, danielbachhuber, pento.
See #45065.

This ticket was mentioned in Slack in #core-js by omarreiss. View the logs.


14 months ago

#32 @pento
14 months ago

In 43812:

Styles: Add helper functions for loading block styles.

Blocks are able to register styles that used in the editor and the frontend, or only in the editor. These functions ensure the correct styles are loaded in the correct place.

See #45065.

#33 @atimmer
12 months ago

In 43978:

Build tools: Combine webpack config files.

This prepares us for building the Gutenberg packages.

Merges [43687] to trunk.
See #45065.

#34 @atimmer
12 months ago

In 44111:

Build tools: Upgrade webpack to version 4.

  • Minification is done by uglify, so disable that in the media build.
  • The webpack boilerplate has changed, which explains the changes in the build files.
  • ModuleConcatenationPlugin is enable by default for production builds so we don't have to specify that ourselves.

Merge notes: In trunk uglify isn't run on the media files after webpack, so webpack does need to do that. Newer webpack versions use terser-webpack-plugin as the default minification. Use the uglifyjs-webpack-plugin plugin to maintain the same behavior as before. We can look into terser as a minifier later.

Merges [43688] to trunk.
See #45065.

#35 @atimmer
12 months ago

In 44112:

Build tools: Build @wordpress packages with webpack.

We decided to split the media webpack config into it's own file. The
main webpack config then combines this file with the packages config.

Include vendor scripts by copying them. We copy the minified files if
they are available. If they aren't available we minify the original
files ourselves.

Props omarreiss, herregroen, gziolo, youknowriad, netweb, adamsilverstein.
Merges [43719] to trunk.
See #45065.

#36 @desrosj
12 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 44114:

Script loader: Register @wordpress scripts.

This allows the packages to be consumed by plugins and core itself.
The code has been based on the work done in the Gutenberg plugin.

We've added an array with all the packages and the vendor packages to
loop through. This sets a convention so all packages will be
registered in the same way. This array can eventually be generated by
a webpack plugin.

We need to register TinyMCE explicitly. Previously TinyMCE was used
by inserting custom <script> tags into the relevant admin pages.
This is not suitable for the new editor, so we need to explicitly
register TinyMCE. We could, in the future, refactor the custom
<script> tags to use the registered TinyMCE script instead.

Polyfills are inserted into the page only when necessary using
document.write.

Props omarreiss, herregroen, youknowriad, gziolo, atimmer.

Merges [43723] to trunk.

Fixes #45065.

#37 @desrosj
12 months ago

In 44115:

Script loader: Adjust JS packages registration.

Adjusts the packages registration after [43723]:

Combine the different registration functions into one wp_default_packages function. To reach this goal move the prefix logic into a function so it can be called from different locations. Use a static variable there to prevent duplicate inclusion of version.php.

Call this function from the wp_default_scripts action by registering it as a default filter.

Combine some of the logic in _WP_Editors::print_tinymce_scripts into wp_register_tinymce_scripts. The logic to force an uncompressed TinyMCE script file stays in _WP_Editors::force_uncompressed_tinymce because that logic is very specific to the classic editor.

The script handle wp-tinymce is now a dependency of the editor script handle. In combination with the previous item, this makes the classic editor work.

Adjust the syntax of the script paths to be more consistent with other WordPress code.

Always use "production" mode for the media files to prevent people from inadvertently committing development files.

Props pento, omarreiss, atimmer.

Merges [43738] into trunk.

Fixes #45065.

#38 @desrosj
12 months ago

In 44119:

Script loading: Fix regression after [43738].

After [43738], TinyMCE would be loaded earlier than before, which makes filters run at a different time relative to the loading of TinyMCE. Fix this by calling wp_print_scripts at the location where TinyMCE would previously be inserted as a <script> tag in the page.

Also, an TinyMCE translation related <script> that was mistakenly removed in [44115].

Props azaozz, omarreiss, swisspidy, atimmer.

Merges [43753], [43754] to trunk.

Fixes #45065.

#39 @pento
12 months ago

In 44143:

Script Loader: Fix metadata being registered as an inline script.

[43723] included script metadata for the wp-polyfill script that was being registered as an inline script.

Merges [43802] from the 5.0 branch to trunk.

See #45065.

#40 @jeremyfelt
12 months ago

In 44150:

REST API: Add endpoints for blocks.

WP_REST_Block_Renderer_Controller allows rendering of server-side rendered blocks, whilst WP_REST_Blocks_Controller allows retrieving of reusable blocks.

Merges [43805] and [43806] from the 5.0 branch to trunk.

Props desrosj, danielbachhuber, pento, Presskopp, swissspidy.
See #45065, #45098.

#41 @desrosj
12 months ago

In 44157:

Styles: Add helper functions for loading block styles.

Blocks are able to register styles that used in the editor and the frontend, or only in the editor. These functions ensure the correct styles are loaded in the correct place.

Props pento.

Merges [43812] to trunk.

See #45065.

Note: See TracTickets for help on using tickets.