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

Bundle the Interactivity API privately in WordPress Core #5195

Conversation

luisherranz
Copy link
Member

@luisherranz luisherranz commented Sep 12, 2023

What

Prepare WordPress Webpack configuration to bundle the interactive blocks (Navigation, File, Search, Image and Query) using a private version of the Interactivity API.

Why

WP 6.4 will include Gutenberg up to the 16.7 version, which includes the new Image Lightbox, an enhanced pagination for the Query block and accessibility fixes for the Navigation block, all of them powered by the Interactivity API. Although the Interactivity API is still in the proposal phase, we can include a private version that only Core blocks can access in WP 6.4 to release these features/enhancements.

Please note that the Interactivity API is still likely to change before the final public version is released, but by keeping it private to Core blocks in WP 6.4, we can still modify it before it's public. Furthermore, if the proposal is finally rejected for some reason, we just have to replace the Interactivity API implementation of the Core blocks with whatever new approach is adopted instead.

How

By modifying the Webpack configuration of the frontend (view.js) files to treat @wordpress/interactivity as a private module that is moved into its own chunk so it can be shared by all of them.

I'm porting the changes that I tested in Gutenberg itself and can be viewed in this PR:

Testing instructions

The latest versions of @wordpress/block-library and @wordpress/interactivity have been included in this PR to simulate a sync and be able to test it. Those package.json modifications should be removed before merging this PR.

You will need to test this using both WordPress and Gutenberg projects.

In the WordPress project:

  • Run npm install to update the @wordpress/block-library and install @wordpress/interactivity.
  • Run npm run build to generate the new files.

In the Gutenberg project:

  • Open a Gutenberg project and checkout the release/16.6 branch, which is the same version than the one installed from npm.
  • Run npm install and npm run build to generate the rest of the files.
  • Open the website and add some interactive blocks.
  • Go to the frontend
  • Inspect the block view files (gutenberg/build/block-library/blocks/navigation/view.min.js).
  • You should see something like this at the top: const e = window.wp.interactivity
  • Inspect the Interactivity API runtime (gutenberg/build/interactivity/index.min.js).
  • You should see something like this at the bottom: window.wp = window.wp || {}).interactivity = e

That is the public version of the Interactivity API.

Now move the generated files from WordPress to Gutenberg so we can test that they work:

  • Copy build/wp-includes/js/dist/interactivity.min.js from WordPress.
  • Paste it as build/interactivity/index.min.js in Gutenberg
  • Copy build/wp-includes/blocks/navigation/view.min.js from WordPress.
  • Paste it as build/block-library/blocks/navigation/view.min.js in Gutenberg
  • Repeat the last steps for the image, file and query blocks. Pay attention to the view filenames, some of them have not been renamed yet and are named view-interactivity.js

Now test that the files work correctly:

  • Go to the frontend
  • Inspect the block view files (gutenberg/build/block-library/blocks/navigation/view.min.js).
  • You should see something like this at the top: self.__WordPressCorePrivateInteractivityAPI__
  • Inspect the Interactivity API runtime (gutenberg/build/interactivity/index.min.js).
  • You should see something like this at the bottom: self.__WordPressPrivateInteractivityAPI__

That is the private version of the Interactivity API.

Tasks

Before merging this PR…

  • Revert the package.json changes (this commit: 940d239)
  • Rename the view files to match the final names in Gutenberg (which should all be view.js).
  • Add the Search block to the list of view files.

I haven't opened a Trac ticket yet because I'm not sure if/when I should do it. If I have to open one, let me know.

@aaronjorbin
Copy link
Member

I haven't opened a Trac ticket yet because I'm not sure if/when I should do it. If I have to open one, let me know.

Please open one. In general, they should be opened before a PR. as noted in the GitHub Pull Requests for Code Review important notes:

Pull requests on GitHub are not monitored. No one will be checking for new pull requests regularly. Pull requests must be attached to a Trac ticket to be considered for inclusion in WordPress Core.

Copy link
Contributor

@tellthemachines tellthemachines left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

These changes will need to be added simultaneously with the npm package updates, so they can be covered by the Trac ticket for those updates (to be created). At the time we should add everything in the same PR to make things easier for testing purposes.

tools/webpack/blocks.js Outdated Show resolved Hide resolved
@gziolo
Copy link
Member

gziolo commented Sep 13, 2023

Some questions about the parts I'm not sure about:

  • I see that @wordpress/interactivity is referenced through the source file: node_modules/@wordpress/interactivity/src/index.js, while view files for blocks are referenced through the files previously processed by the default WordPress Babel config: node_modules/@wordpress/block-library/build-module/query/view. What's the reason for the difference? Note, that view files in this setup get transpiled twice with two different configurations.
  • How is going ./js/dist/interactivity${suffix}.js to be registered and enqueued?
  • DependencyExtractionPlugin uses useDefaults: false setting that essentially will never add an item to the deps array in the *.asset.php file. How do you plan to connect the view files with the shared chunk so they get enqueued in the correct order?

It’s been a constant challenge to coordinate webpack changes that need to be applied in both the Gutenberg plugin and WordPress core. This one seems even more complex because there is a completely new Babel config required for the front end code.

@luisherranz
Copy link
Member Author

I see that @wordpress/interactivity is referenced through the source file: node_modules/@wordpress/interactivity/src/index.js, while view files for blocks are referenced through the files previously processed by the default WordPress Babel config

Good catch. We need to use src instead of build-module for @wordpress/interactivity because, as you say, the files in the build-module folder have been processed with the default Gutenberg Babel config. In @wordpress/interactivity we need to replace the JSX with calls to h from Preact, but in the build-module folder the JSX has already been replaced with calls to createElement from @wordpress/element, so it's too late.

As for the view.js files, we are not doing anything special in Gutenberg, which means they are processed twice, so I thought it'd be better to keep it as close as possible to the version in Gutenberg.

In the future, however, we will want to allow JSX via Preact in the view.js files, so we will want to process those files with a different Webpack configuration. That would break view.js files of external plugins that contain JSX and rely on @worpdress/element, but I thing we can rely on the existence of "supports.interactivity" to process the files differently. Anyway, that's not important at this point, so we can do it later.

Does that make sense?

How is going ./js/dist/interactivity${suffix}.js to be registered and enqueued?

In Gutenberg, it's registered automatically, so I assumed it would be registered automatically in WordPress as well. Is that not the case?

As for enqueueing, we'll do it in the index.php file of each block with this code. We'll merge a PR with that before the 16.7 RC.

By the way, everything is tracked in this Tracking Issue. I should have mentioned it in the description.

How do you plan to connect the view files with the shared chunk so they get enqueued in the correct order?

Webpack takes care of the order execution and it doesn't matter which chunk loads first. Actually, these types of bundles used to be loaded with async instead of defer 🙂

@luisherranz
Copy link
Member Author

luisherranz commented Sep 13, 2023

At the time we should add everything in the same PR to make things easier for testing purposes.

Makes sense. Does that mean that I don't need to open a Trac ticket for this, or do I?

@gziolo
Copy link
Member

gziolo commented Sep 13, 2023

How is going ./js/dist/interactivity${suffix}.js to be registered and enqueued?

In Gutenberg, it's registered automatically, so I assumed it would be registered automatically in WordPress as well. Is that not the case?

It's rather different in WordPress core, but you need to double check.

https://github.com/WordPress/gutenberg/blob/b05bee7e2e8eefb2be06c228002e4f9783847747/lib/client-assets.php#L190

vs

$assets = include ABSPATH . WPINC . "/assets/script-loader-packages{$suffix}.php";
foreach ( $assets as $file_name => $package_data ) {

The runtime chunk for Interactivity might not be included in the combined asset file because it's excluded from entry points.

As for enqueueing, we'll do it in the index.php file of each block with WordPress/gutenberg#54311 (comment). We'll merge a PR with that before the 16.7 RC.

That can be automated by the DependencyExtractionWebpackPlugin. I see that the flag useDefaults is set to false, but there is still a way to detect import paths to the bundled Interactivity API runtime and push the script handle that it's going to be registered at in WordPress core. See requestToHandle.

@luisherranz
Copy link
Member Author

That can be automated by the DependencyExtractionWebpackPlugin. I see that the flag useDefaults is set to false, but there is still a way to detect import paths to the bundled Interactivity API runtime and push the script handle that it's going to be registered at in WordPress core. See requestToHandle.

I looked at the documentation but I couldn't find a way to avoid the replacement of @wordpress/interactivity with wp.interactivity and still get wp-interactivity listed as a dependency. But if I missed something please let me know 🙂

It's rather different in WordPress core, but you need to double check.

Ok, we'll take a look at that file. Thank you!

The runtime chunk for Interactivity might not be included in the combined asset file because it's excluded from entry points.

Yes, the runtime is included in the shared chunk. You can test it by running npm run build in this PR and inspecting the generated files, or by testing it with the instructions that I gave in Gutenberg to see that the interactive blocks work fine.

@tellthemachines
Copy link
Contributor

Does that mean that I don't need to open a Trac ticket for this, or do I?

You don't need a Track ticket for this; it will be part of the package update ticket.

@gziolo
Copy link
Member

gziolo commented Sep 14, 2023

That can be automated by the DependencyExtractionWebpackPlugin. I see that the flag useDefaults is set to false, but there is still a way to detect import paths to the bundled Interactivity API runtime and push the script handle that it's going to be registered at in WordPress core. See requestToHandle.

I looked at the documentation but I couldn't find a way to avoid the replacement of @wordpress/interactivity with wp.interactivity and still get wp-interactivity listed as a dependency. But if I missed something please let me know 🙂

I can confirm that you can't use requestToHandle without having the same path converted to the external first with requestToExternal. I checked how I did it before in the Gutenberg plugin, and I used an experimental flag to inject wp-interactivity as a dependency for view scripts. I recreated it in a branch and the related commit would look as follows:

WordPress/gutenberg@d381d10

The usage in the webpack config would be:

// Same values as keys in config.entry.
const interactiveBlockFiles = [
	'file/view',
	'image/view',
	'navigation/view',
	'query/view',
];

// Later in the config.
new DependencyExtractionPlugin( {
	injectPolyfill: false,
	useDefaults: false,
	__experimentalInteractiveBlockFiles: interactiveBlockFiles,
} )

Example asset file for the File block:

Screenshot 2023-09-14 at 12 20 01

@gziolo
Copy link
Member

gziolo commented Sep 14, 2023

Thinking more broadly about the use case that we have here - shared runtime chunk, I think we should also consider changing the behavior in the Dependency Extraction Webpack Plugin:

https://github.com/WordPress/gutenberg/blob/f92f9e28ba4fb2e435cb89d5b327b707cb805ba7/packages/dependency-extraction-webpack-plugin/lib/index.js#L188C4-L192

I think this code could get refactored to as it's no longer the externals, but it could cover also shared chunks like the runtime:

const processModule = ( { userRequest } ) => {
	chunkDeps.add( this.mapRequestToDependency( userRequest ) );
};

then something like the following could work:

new DependencyExtractionPlugin( {
	injectPolyfill: false,
	useDefaults: false,
	requestToHandle( request) {
		if ( '@wordpress/interactivity' ) {
			return 'wp-interactivity';
		}
	} 
} )

@luisherranz
Copy link
Member Author

Taking into account that we will probably only need this for the WP 6.4 version, I thought that we could avoid modifying DependencyExtractionPlugin (or introducing experimental options).

What do you think about creating an ad-hoc Webpack plugin to generate the assets file? Something like this: 1f59ead (I only tested the JavaScript side). That way, we can easily remove the plugin once we don't need it anymore.

@gziolo
Copy link
Member

gziolo commented Sep 15, 2023

What do you think about creating an ad-hoc Webpack plugin to generate the assets file? Something like this: 1f59ead (I only tested the JavaScript side). That way, we can easily remove the plugin once we don't need it anymore.

You don't necessarily need the asset file to register the temporary script (assuming it's only for WordPress 6.4) with runtime containing the code for Interactive API. It doesn't have any dependencies, and the default version used for registered scripts defaults to the current WP version – 6.4.0, 6.4.1, etc.

The following might work, too (I'm not sure if add uses the same defaults as wp_register_script):

$scripts->add(
	'wp-interactivity',
	'/wp-includes/js/dist/interactivity.js'
);

For view files, I was only referring to the fact that they don't list their dependency on the runtime, so assuming the example it would be array( 'dependencies' => array( 'wp-interactivity' ) ).

Anyway, there are multiple ways it can be done, so feel free to pick one that is the easier to maintain in the full release cycle.

@luisherranz
Copy link
Member Author

the default version used for registered scripts defaults to the current WP version – 6.4.0, 6.4.1

Ok, I thought the hash was a standard in WP Core and I didn't want to change that for this script. Then it's much easier, thanks Greg 🙂

For view files, I was only referring to the fact that they don't list their dependency on the runtime, so assuming the example it would be array( 'dependencies' => array( 'wp-interactivity' ) ).

My idea was to put that in the block themselves (block-library), but maybe it's easier to add it here. It's not really needed in Gutenberg.

@gziolo
Copy link
Member

gziolo commented Sep 18, 2023

Ok, I thought the hash was a standard in WP Core and I didn't want to change that for this script. Then it's much easier, thanks Greg 🙂

We had to automate managing all the entry points and their dependencies because updating it manually was error-prone and took forever to update on every change in Gutenberg. The nice side-effect was the unique versions generated by webpack that allowed to improve the caching for JS files, as the version only changes when the content of the file is different between releases. So it's particularly handy during bug fix or security releases. It isn't mandatory though and it shouldn't be a big deal for a single entry point if that is meant to exist only for 6.4 cycle.

@luisherranz
Copy link
Member Author

Ok, I think this should be ready now in case someone wants to take another look.

I finally added the wp-interactivity dependency in Gutenberg. It felt easier that way:

mikachan added a commit to mikachan/wordpress-develop that referenced this pull request Sep 20, 2023
@luisherranz
Copy link
Member Author

Closing this, as the works continues now in the official PR:

mikachan added a commit to mikachan/wordpress-develop that referenced this pull request Sep 26, 2023
ockham pushed a commit to ockham/wordpress-develop that referenced this pull request Sep 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants