WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.51k stars 4.2k forks source link

Update and de-duplicate Caniuse data routinely, at least for releases #33344

Open kraftner opened 3 years ago

kraftner commented 3 years ago

What problem does this address?

Currently Caniuse Data is apparently not updated and de-duplicated routinely which can lead to multiple different versions of the data being used across different dependencies:

See for example the current version for WordPress 5.7:

https://github.com/WordPress/gutenberg/blob/wp/5.7/package-lock.json https://github.com/WordPress/gutenberg/blob/bc1c2608b705f479f95c4fa1ade52fd8ad30e0fe/package-lock.json

(Unfortunately I can't link to the exact line due to the size of the file but if you search for the exact string "caniuse-lite": { inside https://raw.githubusercontent.com/WordPress/gutenberg/bc1c2608b705f479f95c4fa1ade52fd8ad30e0fe/package-lock.json you'll see that 4 different versions of the caniuse-lite data are used)

Among other things this makes it hard to do things like do isolated rebuilds of block CSS.

In any case doing this is also what the browserslist package recommends.

What is your proposed solution?

In any case there should only ever be one specific version of caniuse-lite used across all packages at all time.

getdave commented 3 years ago

@nerrad this might be one for #core-js chat soon?

nerrad commented 3 years ago

Yup, I added to the agenda for the next chat.

gziolo commented 3 years ago

It feels like some sort of integration with the release process of the Gutenberg plugin should ensure that the same version caniuse-lite is used by all tools that have it defined as a deep dependency. @youknowriad, do you think we could add npx browserslist@latest --update-db call at the time the RC of the Gutenberg plugin is published with the GitHub workflow? The result of the command could be committed together with the plugin version bump.

In the WordPress core, we could run this command as a step after wp-packages-update:

https://github.com/WordPress/wordpress-develop/blob/ba98780ed5c8849d39f00695125f0f9315c2fca0/package.json#L176

kraftner commented 3 years ago

This sounds generally sane to me.

@gziolo Sorry for my ignorance on the process, but for WordPress Core - when is wp-package-update run? Is it only when packages are updated or also when new ones are added? Just checking so nothing slips through the cracks...

gziolo commented 3 years ago

@kraftner, wp-packages-update is executed only when WordPress packages (from Gutenberg) are updated.

kraftner commented 3 years ago

Okay, so if someone installs a new package for some reason we might still end up with the original issue, no?

gziolo commented 3 years ago

Okay, so if someone installs a new package for some reason we might still end up with the original issue, no?

I don't think it ever becomes an issue because every major WordPress release contains a few Gutenberg plugin releases where most of them translate to npm publishing. In effect, wp-packages-update is executed in WordPress core several times per a major release.

gziolo commented 3 years ago

I played with the command a bit in https://github.com/WordPress/gutenberg/pull/34685. One issue I noticed is that npx browserslist@latest --update-db changes the formatting of the lock file ...

gziolo commented 2 years ago

It’s now a grunt task in WordPress core and the plan is to enable it by default when syncing Gutenberg changes in WP trunk. See https://github.com/WordPress/wordpress-develop/blob/e17a83df22f57016dec8e70c9a35f2cf113f2e4a/Gruntfile.js#L1684.

youknowriad commented 2 years ago

I think it's important to avoid running this command on "patch" releases though of WP as otherwise, we'd be changing the supported browsers of a WP release on patch release.

gziolo commented 2 years ago

I think it's important to avoid running this command on "patch" releases though of WP as otherwise, we'd be changing the supported browsers of a WP release on patch release.

Yes, it's optional at the moment. I think the best way to move forward is to run it only when in trunk branch and when the dist-tag option passed to grunt is set to latest.

kraftner commented 2 years ago

As mentioned in the original issue this is a two-part issue: Updating browser data and de-duplicating. So concerning the updating I agree that it probably shouldn't be done for patch releases. But for the de-duplication I am not so sure if that is something that should be skipped for patch releases.

gziolo commented 2 years ago

So concerning the updating I agree that it probably shouldn't be done for patch releases. But for the de-duplication I am not so sure if that is something that should be skipped for patch releases.

@adamziel, any thoughts on how we can update the handling for browserslist update in WP core so it aligns with what @kraftner highlights in this issue? See, also my previous comment https://github.com/WordPress/gutenberg/issues/33344#issuecomment-1120169927 where I commented about possible changes to the default handling of when to trigger the update.

adamziel commented 2 years ago

@gziolo I do agree with your conclusions here. For the purposes of sync-gutenberg-packages automation in wordpress-develop, let's restrict running browserslist:update to the latest dist tag on trunk: https://github.com/WordPress/wordpress-develop/pull/2679/commits/848ba383a7427c4ffbc6f76011c7c5053ceaad25

kraftner commented 2 years ago

@gziolo Wouldn't it then still be possible that duplicates of caniuse-lite appear on patch releases?

gziolo commented 2 years ago

Wouldn't it then still be possible that duplicates of caniuse-lite appear on patch releases?

Yes, that would be useful. Do you know how this can be done without upgrading the version?