-
Notifications
You must be signed in to change notification settings - Fork 799
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
Blocks: require wp-polyfill only when really needed #39452
Comments
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! |
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 |
The plugin lives inside the 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). |
We support the current and previous versions of WordPress, so currently 6.5 and 6.6. Is that the same versions of |
I don't know how As far as I know (and can tell from live sites), Gutenberg doesn't override
But perhaps I'm missing something? |
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 If we switch to the new method for the standard config, enabling
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 Does it sound like I got anything wrong in there? |
I think everything is correct in that analysis @anomiex 👍
One idea here is that you may be able to add the magic comment yourself to opt-in. |
I've got looking at this on my list, but for the moment I'm waiting on WordPress/gutenberg#65582 to avoid having to mess with the terser config. |
Currently, all our blocks depend on the fairly large
wp-polyfill
package, even if they might not need it:jetpack/projects/plugins/jetpack/class.jetpack-gutenberg.php
Line 559 in 70d1104
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 ifwp-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 likewp-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 thewp-polyfill
in theview.asset.php
file only when truly needed.Possible todos
dependency-extraction-webpack-plugin
wp-polyfill
dependencycc @oskosk @kraftbj @jeherve
The text was updated successfully, but these errors were encountered: