Remove importmap polyfill

[swissspidy]

In #60348 / [57492] a polyfill was added for import maps (type="importmap").

From what I can see, all browsers supported by WordPress actually already do support import maps, so there isn't really a reason to include this polyfill.

For reference:

• ​caniuse.com data
• ​WordPress browser support policy

If you take the list of supported browsers from that page (also available ​in this browserslist config) and ​test it on the browserslist website, you'll only get browsers that already support import maps.

Thus, I propose removing this polyfill again by not printing it anymore in \WP_Script_Modules::print_import_map()

Note: actual performance impact is minimal as on all these browsers, no extra script is loaded. It's just a bit of unnecessary inline JS sent over the wire.

[luisherranz]

The usage of Safari < 16.3 on iOS is still 3.6%, and WordPress is meant to support any browser with usage bigger than 1%.

[swissspidy]

Replying to luisherranz:

The usage of Safari < 16.3 on iOS is still 3.6%, and WordPress is meant to support any browser with usage bigger than 1%.

My understanding is that means a single browser version bigger than 1%, not a range of versions added together being bigger than 1%. Otherwise our browserslist config is incorrect. And if you check the browserslist config above, you‘ll see those Safari versions not being liated.

Look at individual iOS Safari versions (I‘m only listing the ones without importmap support):

16.3 - 0.42% usage
16.2 - 0.24% usage
16.1 - 0.49% usage

All below 1%

[swissspidy]

Quoting @gziolo:

It looks like we don't need the polyfill for the import map, taking into account the list of currently supported browsers.

[gziolo]

I see the full context now. I'm not sure what's the correct way to interpret the browserslist settings vs the policy that WordPres has. However, realistically looking at where we are, we have been using these settings since WordPress 5.0 so I would say it's fair to say it should be the norm how @swissspidy has interpreted them.

@youknowriad and @desrosj, what do you think?

[luisherranz]

Look at individual iOS Safari versions (I‘m only listing the ones without importmap support):
16.3 - 0.42% usage
16.2 - 0.24% usage
16.1 - 0.49% usage
All below 1%

That doesn't make any sense to me. It's totally arbitrary. Browser vendors can release new versions each week or each year. Also, why minor and not major versions? The policy doesn't say anything about this. If we choose major versions, iOS 16 > 1%.

In my opinion, for the frontend, it would be better to use global usage. Something like global > 95%. For the admin is a different story.

[swissspidy]

Also, why minor and not major versions? The policy doesn't say anything about this.

FWIW I don't think Safari really treats those as minor versions, they don't follow semver.

In my opinion, for the frontend, it would be better to use global usage. Something like global > 95%. For the admin is a different story.

Just noting that this browserslist config and browser support policy page have been like this for years, ever since Gutenberg got introduced. My proposal is based on this existing documentation.

If we want to change the WP browser support policy I suggest writing a detailed make/core proposal, including a draft PR for the browserslist config so we can vet bundle size differences.

[oglekler]

I am moving this ticket to the next milestone for further discussion.

[desrosj]

I agree with @swissspidy's interpretation of the list of supported browsers and the responses/reasoning shared. I looked back and the > 1% was introduced in [41062] (see #37651), and this appears to be how it was interpreted at the time and since.

I think that the ​link included in the ticket description also demonstrates this understanding is generally accepted as the best practice, and it's how it's interpreted by the browsersList tool as well.

Also, why minor and not major versions? The policy doesn't say anything about this. If we choose major versions, iOS 16 > 1%.

Unless I'm misreading the documentation, I'm not sure that Browserslist even provides a way to dynamically specify full ranges like you describe @luisherranz. In order to accomplish this, we'd have to manually list out the versions we want to support, which would also require us to monitor browser usage ourselves and updated the configuration.

I am in favor of removing the polyfill in this instance. Normally I would advocate that we leave the script itself in Core making it available through a registered script handle like with wp-polyfill-element-closest or wp-polyfill-node-contains, but ​usage appears to essentially 0% in the plugin directory.

Removing and calling this out as a bulleted list item in the "Miscellaneous 6.7 developer focused changes" dev note seems sufficient.

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

[swissspidy]

Whoops, I missed that you added the good-first-bug keyword!

I already created a PR for it. Sorry about that.

@swissspidy Is there any reason not to remove the other changes in the related commit? It looks like the wp-polyfill-importmap handle is not being used anywhere, and the es-module-shims dependency where wp-polyfill-importmap is built from also appears not to be used.

Historically we haven't removed old polyfills even though they are no longer used. There's still a bunch of them registered here:

​https://github.com/WordPress/wordpress-develop/blob/324aeb7485b800aef96d4c68b9f1ca36dbca1b7c/src/wp-includes/script-loader.php#L90-L126

If we look at them holistically I wouldn't mind cleaning up a little and replace the scripts with blanks or something.

This way we could tidy up package.json and our build script a bit as well.

Or are you suggesting only removing this one completely, because it's so new and probably completely unused? And leave the rest as-is?

Or are you suggesting only removing this one completely, because it's so new and probably completely unused? And leave the rest as-is?

Correct. There's only ​5 occurrences of this script handle in the plugin directory, 2 of which are Gutenberg itself. I'd love to not have another obscure polyfill to update every four-ish months.

I'd be open to reviewing this policy more holistically. But should probably do that separately.

And one of them is from a broken translation file that contains a 404 page's HTML markup 😅

Just pushed these changes now

[desrosj]

importmap related: #61734, #56313.

[swissspidy]

In 58952:

Script Loader: Remove importmap polyfill.

The polyfill was added in [57492], but all browsers supported by WordPress already support import maps.

This not only disables the polyfill, but completely removes it as it was only added recently and there is no usage outside of core.

Props swissspidy, desrosj, luisherranz, gziolo.
Fixes #60970.

Committed in https://core.trac.wordpress.org/changeset/58952

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

[swissspidy]

In 58953:

Docs: Remove @global annotation after [58952].

Props mukesh27.
See #60970.

https://core.trac.wordpress.org/changeset/58953