Make WordPress Core

Opened 7 weeks ago

Closed 6 weeks ago

Last modified 8 days ago

#60962 closed enhancement (fixed)

Remove now unnecessary polyfill dependencies

Reported by: swissspidy's profile swissspidy Owned by: swissspidy's profile swissspidy
Milestone: 6.6 Priority: normal
Severity: normal Version:
Component: Script Loader Keywords: has-patch commit needs-dev-note
Focuses: performance Cc:

Description

In [43903], wp-polyfill was added as a direct dependency of the react asset.

This brought over this Gutenberg PR into core ahead of the 5.0 release.

Apparently, the reason for this was IE 11. React 16 used to require a few runtime JS features that were missing in IE11 and thus needed to be polyfilled. So when using the externalized React version provided by WordPress, it was important for wp-polyfill to be loaded before React.

However, times have changed.

WordPress core dropped IE 11 support in 2021.

React has since been updated to version 18.

Developers using @wordpress/scripts to build their assets need to manually include the `wp-polyfill` dependency if they need polyfills.

Yet, if a plugin loads React for something — for example on the frontend — the wp-polyfill script is still unnecessarily enqueued.

At least the wp-polyfill.min.js size decreased from 115 KB in WP 6.4 to 39 KB in WP 6.5. But that is still 39 KB of unnecessary JavaScript that is loaded for these cases.

I therefore propose removing the wp-polyfill dependency from react. Ideally this accompanied by a dev-note.

Attachments (2)

Screenshot 2024-04-16 at 3.21.00 PM.jpg (250.2 KB) - added by donnapep 5 weeks ago.
regenerator-runtime Error
Screenshot 2024-04-16 at 3.21.11 PM.jpg (64.7 KB) - added by donnapep 5 weeks ago.
Related code

Download all attachments as: .zip

Change History (25)

#1 @youknowriad
7 weeks ago

The reasoning makes sense to me.

#2 @gziolo
7 weeks ago

Yes, we shouldn't need it anymore. In the editor, we will have the polyfill coming from other packages anyway, but I think React doesn't need it these days.

#3 @swissspidy
7 weeks ago

  • Milestone changed from Future Release to 6.6

Similarly, is wp-polyfill-inert still needed? It was added in #57492

Looking at https://caniuse.com/mdn-html_global_attributes_inert and our browserslist config, all browsers seem to support it.

So maybe something we can drop too?

This ticket was mentioned in PR #6372 on WordPress/wordpress-develop by @swissspidy.


7 weeks ago
#4

  • Keywords has-patch added; needs-patch removed

To-do:

  • [ ] Maybe remove wp-polyfill-inert too

Trac ticket: https://core.trac.wordpress.org/ticket/60962

#5 @youknowriad
7 weeks ago

Indeed, we can remove "inert" but I wonder if we should keep the an empty script handle around for third-parties.

#6 @swissspidy
7 weeks ago

Indeed, we can remove "inert" but I wonder if we should keep the an empty script handle around for third-parties.

I think for now we can probably simply not make it a dependency of wp-polyfill anymore, so that it doesn't get automatically loaded for everyone using wp-polyfill.

Related to that, does regenerator-runtime need to be loaded all the time as well? Can this be loaded on demand instead by projects explicitly enqueuing it if they need it?

#7 @adamsilverstein
6 weeks ago

+1 Thanks for opening this @swissspidy!

This ticket was mentioned in Slack in #core-performance by adamsilverstein. View the logs.


6 weeks ago

#9 @youknowriad
6 weeks ago

Related to that, does regenerator-runtime need to be loaded all the time as well? Can this be loaded on demand instead by projects explicitly enqueuing it if they need it?

I think regenerator-runtime is there to ensure generator support but generators are now supported by all the browsers that we support, so I think it's probably not needed anymore https://caniuse.com/?search=generators

We need to check in the "built files" if generators are kept as they are (because there could be some option to enable/disable in babel)

#10 @swissspidy
6 weeks ago

@youknowriad How can this be checked? I'm assuming this applies only to Gutenberg packages with @babel/runtime as a dependency?

#11 @youknowriad
6 weeks ago

@swissspidy So I just checked using a generator in some random Gutenberg script and this is the output that I've got

function* myGenerator() {
  yield 1;
  yield 2;
  yield 3;
}
window.myGenerator = myGenerator();

It seems the output kept the generator as is, so I'd say that it's probably fine to remove the regenerator-runtime dependency at this point from all of our scripts.

#12 @swissspidy
6 weeks ago

  • Keywords commit added
  • Owner set to swissspidy
  • Status changed from new to accepted
  • Summary changed from Remove `wp-polyfill` dependency from `react` to Remove now unnecessary polyfill dependencies

#13 @swissspidy
6 weeks ago

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

In 57981:

Script Loader: stop enqueueing some now obsolete polyfills.

Stop enqueueing polyfills such as wp-polyfill-inert (for the inert attribute) and regenerator-runtime (for generator functions), as they are no longer needed, considering the WordPress project's browser support policy.

In addition to that, wp-polyfill (essentially core-js) is no longer enqueued as a dependency of react. This was added in [43903] to ensure compatibility with IE 11, which is no longer supported by WordPress. Developers requiring wp-polyfill need to manually add it as a dependency for their scripts.

Props swissspidy, flixos90, adamsilverstein, youknowriad, gziolo.
Fixes #60962.

#16 @swissspidy
6 weeks ago

  • Keywords needs-dev-note added

Something for a misc dev note

#17 @youknowriad
6 weeks ago

I've been thinking a bit more here. It is possible that some plugins are declaring wp-polyfill as a dependency but are still compiling generators to use the generator runtime. It would be a small breaking change for these plugins.

To be honest, I have no idea if there are such plugins and their numbers, but thought I'd mention.

We could advise them to:

  • Update their build tools to not transpile generators.
  • Add regenerator-runtime dependency explicitly.

I'd probably recommend handling that in the dev note for now (and keep an eye on feedback).

@donnapep
5 weeks ago

regenerator-runtime Error

#18 @donnapep
5 weeks ago

Dropping a quick note here in case it's useful.

The following error was reported to the Sensei product team. It appears on any Sensei course page on the learn.wordpress.org site:

regenerator-runtime Error
Related code

The source code is here.

I'll take a closer look at the proposed solution to try to resolve.

#19 @youknowriad
5 weeks ago

Any thoughts on that @swissspidy is it severe enough that we should restore the regenerator-runtime (and find some ways to deprecate it) or should we address this more in terms of communication / dev note?

#20 @swissspidy
5 weeks ago

Unless there are more severe reports I still think a dev-note suffices.

I'd say nowadays the majority of WordPress developers use wp-scripts, where this is not an issue.

From what I can see, Sensei uses some Calypso-specific build tooling, so this should be resolved there, either by updating the tooling or manually adding the dependency.

#21 @swissspidy
11 days ago

Do we also need to remove the injectPolyfill line from the webpack configs? 🤔

#22 @swissspidy
8 days ago

In 58162:

Script Loader: Disable injectPolyfill flag in webpack config.

This is a follow-up to [57981] to ensure that wp-polyfill is no longer automatically added as a dependency to package scripts. This avoids unnecessarily loading the polyfill script in places such as the block editor.

See #60962.

#23 @swissspidy
8 days ago

In 58163:

Script Loader: Update failing test after [58162].

See #60962.

Note: See TracTickets for help on using tickets.