Skip to content
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

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

Open
kraftner opened this issue Jul 11, 2021 · 16 comments
Open

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

kraftner opened this issue Jul 11, 2021 · 16 comments
Labels
Good First Issue An issue that's suitable for someone looking to contribute for the first time Needs Dev Ready for, and needs developer efforts npm Packages Related to npm packages [Type] Build Tooling Issues or PRs related to build tooling

Comments

@kraftner
Copy link

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?

  • Either integrate automated runs of npx browserslist@latest --update-db into the build process.
  • Or do something similar that doesn't update but only de-duplicates caniuse-lite based on the above browserslist script.

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

@getdave getdave added [Type] Build Tooling Issues or PRs related to build tooling npm Packages Related to npm packages labels Jul 21, 2021
@getdave
Copy link
Contributor

getdave commented Jul 21, 2021

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

@nerrad
Copy link
Contributor

nerrad commented Jul 21, 2021

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

@gziolo
Copy link
Member

gziolo commented Jul 27, 2021

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
Copy link
Author

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
Copy link
Member

gziolo commented Aug 16, 2021

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

@kraftner
Copy link
Author

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

@gziolo
Copy link
Member

gziolo commented Aug 17, 2021

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
Copy link
Member

gziolo commented Sep 9, 2021

I played with the command a bit in #34685. One issue I noticed is that npx browserslist@latest --update-db changes the formatting of the lock file ...

@gziolo gziolo added Needs Dev Ready for, and needs developer efforts Good First Issue An issue that's suitable for someone looking to contribute for the first time labels Feb 11, 2022
@gziolo
Copy link
Member

gziolo commented May 7, 2022

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.

@gziolo gziolo closed this as completed May 7, 2022
@youknowriad
Copy link
Contributor

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
Copy link
Member

gziolo commented May 7, 2022

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
Copy link
Author

kraftner commented May 9, 2022

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
Copy link
Member

gziolo commented May 9, 2022

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 #33344 (comment) where I commented about possible changes to the default handling of when to trigger the update.

@adamziel
Copy link
Contributor

adamziel commented May 10, 2022

@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: WordPress/wordpress-develop@848ba38

@kraftner
Copy link
Author

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

@gziolo
Copy link
Member

gziolo commented May 10, 2022

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good First Issue An issue that's suitable for someone looking to contribute for the first time Needs Dev Ready for, and needs developer efforts npm Packages Related to npm packages [Type] Build Tooling Issues or PRs related to build tooling
Projects
None yet
Development

No branches or pull requests

6 participants