Automattic / jetpack

Security, performance, marketing, and design tools β€” Jetpack is made by WordPress experts to make WP sites safer and faster, and help you grow your traffic.
https://jetpack.com/
Other
1.59k stars 798 forks source link

Blocks: require wp-polyfill only when really needed #39452

Closed simison closed 1 month ago

simison commented 1 month ago

Currently, all our blocks depend on the fairly large wp-polyfill package, even if they might not need it:

https://github.com/Automattic/jetpack/blob/70d1104596b18f7e16a5b705fa26c27d7581b35f/projects/plugins/jetpack/class.jetpack-gutenberg.php#L559

If I remember right, we added the dependency circa ~2019 after we had several bad breakages on the front end of the site in older Internet Explorer browsers (which we don't support anymore). I haven't actually reviewed our view.js builds to see if wp-polyfill is in any of them currently, but that might also change over time and be error-prone if we ignore it.

Meanwhile, in Gutenberg, there's some work by @sgomes on packages to depend on wp-polyfill only when truly needed:

This is great because now, even if we wouldn't require wp-polyfill for blocks, simply depending on any core package like wp-dom-ready would pull in the dependency anyway.

If I understand the PR correctly, dependency-extraction-webpack-plugin now supports the mechanism out of the box and would conditionally output the wp-polyfill in the view.asset.php file only when truly needed.

Possible todos

cc @oskosk @kraftbj @jeherve

sgomes commented 1 month ago

Thank you for the mention, @simison!

Hey folks! πŸ‘‹

To clarify, there are two parts to the change:

These changes were designed primarily for internal use in Gutenberg, but it should be possible to expose them to other consumers of these packages. It may require further changes, though, depending on how all of these pieces interact.

Happy to help anyone looking into this on the Jetpack side!

anomiex commented 1 month ago

We'd have to change our own Babel config to generate those comments when appropriate, since we don't use Gutenberg's. Is there a Babel plugin to do only that, or is it only embedded inside your @wordpress/babel-preset-default?

sgomes commented 1 month ago

We'd have to change our own Babel config to generate those comments when appropriate, since we don't use Gutenberg's. Is there a Babel plugin to do only that, or is it only embedded inside your @wordpress/babel-preset-default?

The plugin lives inside the babel-preset-default package; it's replace-polyfills.js on the root.

If you're going to be using your own babel config and the Gutenberg-generated polyfills script, then in order for everything to work correctly, you'll need to ensure that:

You should end up with something along the lines of:

{
  presets: [
    // ...
    [ require.resolve( '@babel/preset-env' ), {
      useBuiltIns: 'usage',
      exclude: require( '@wordpress/babel-preset-default/polyfill-exclusions.js' ),
      corejs: require( 'core-js/package.json' ).version,
    } ],
    // ...
  ]
  plugins: [
    // ...
    require( '@wordpress/babel-preset-default/replace-polyfills.js' )
  ]
}

Note that I haven't tested this, so I may have gotten things wrong (like how to address the individual JS files inside the package).

anomiex commented 1 month ago

You use the same version of core-js and browserslist as Gutenberg. This is important so that they have the same browser support info.

We support the current and previous versions of WordPress, so currently 6.5 and 6.6. Is that the same versions of core-js and browserslist as in the copy of Gutenberg bundled in 6.5 or 6.6? Or people might have installed the Gutenberg plugin, do we need to account for that too?

sgomes commented 1 month ago

We support the current and previous versions of WordPress, so currently 6.5 and 6.6. Is that the same versions of core-js and browserslist as in the copy of Gutenberg bundled in 6.5 or 6.6? Or people might have installed the Gutenberg plugin, do we need to account for that too?

I don't know how wp-polyfill ends up making its way from Gutenberg to Core, sorry πŸ˜• Perhaps @sirreal knows more about that?

As far as I know (and can tell from live sites), Gutenberg doesn't override wp-polyfill, and the copy from Core is the one that's always in use. So while there is no perfect way of handling multiple versions, the latest Gutenberg should always have the most up-to-date config, and I think it should be safe to base your config on that:

But perhaps I'm missing something?

anomiex commented 1 month ago

After doing some reading and some looking around, and then some thinking, here's what I think I know.

So, the current state is that we get whatever core-js polyfills are included in whatever version of wp-polyfills is bundled with WordPress and no others.

If we switch to the new method for the standard config, enabling core-js polyfilling but then replacing all the polyfill imports with magic comments, we'll still get whatever core-js polyfills are included in whatever version of wp-polyfills is bundled with WordPress and no others, even if we don't match versions. Specifically:

So it looks like for an initial version we can ignore any version and config matching after all, but we'll need our own custom version of replace-polyfills.js that handles absoluteImports. For packages/search and packages/wordads we'll want to not enable the replace-polyfills.js so we don't accidentally lose polyfills not in wp-polyfills.

Does it sound like I got anything wrong in there?

sirreal commented 1 month ago

I think everything is correct in that analysis @anomiex πŸ‘

For packages/search and packages/wordads we'll want to not enable the replace-polyfills.js so we don't accidentally lose polyfills not in wp-polyfills.

One idea here is that you may be able to add the magic comment yourself to opt-in.

sgomes commented 1 month ago

@anomiex Flawless research, @anomiex, that's all correct! πŸ‘

anomiex commented 1 month ago

I've got looking at this on my list, but for the moment I'm waiting on https://github.com/WordPress/gutenberg/pull/65582 to avoid having to mess with the terser config.