browserslist / update-db

CLI tool to update caniuse-lite to refresh target browsers from Browserslist config
https://browsersl.ist/
MIT License
364 stars 70 forks source link

under PNPM, "npx update-browserslist-db@latest" & "pnpm up caniuse-lite" updates other dependencies #28

Open MarArMar opened 1 year ago

MarArMar commented 1 year ago

In a Turborepo :

I got the notification when I ran pnpm run dev --filter ProjectName

Browserslist: caniuse-lite is outdated. Please run:
  npx update-browserslist-db@latest
  Why you should do it regularly: https://github.com/browserslist/update-db#readme

I ran

running npx update-browserslist-db@latest

But it:

At the end, I had a broken project

The behavior of "update-browserslist-db@latest" should be strictly limited to its purpuse : updating caniuse-lite/Browserslist and nothing else

Note : I had the notification because I had

autoprefixer : 10.4.14

In pnpm lock :

  /postcss-normalize-unicode@5.1.1(postcss@8.4.24):
    resolution: {integrity: sha512-qnCL5jzkNUmKVhZoENp1mJiGNPcsJCs1aaRmURmeJGES23Z/ajaln+EPTD+rBeNkSryI+2WTdW+lwcVdOikrpA==}
    engines: {node: ^10 || ^12 || >=14.0}
    peerDependencies:
      postcss: ^8.2.15
    dependencies:
      browserslist: 4.21.7
      postcss: 8.4.24
      postcss-value-parser: 4.2.0
    dev: false

  /autoprefixer@10.4.14(postcss@8.4.24):
    resolution: {integrity: sha512-FQzyfOsTlwVzjHxKEqRIAdJx9niO6VCBCoEwax/VLSoQF29ggECcPuBqUMZ+u8jCZOPSy8b8/8KnuFbp0SaFZQ==}
    engines: {node: ^10 || ^12 || >=14}
    hasBin: true
    peerDependencies:
      postcss: ^8.1.0
    dependencies:
      browserslist: 4.21.7
      caniuse-lite: 1.0.30001492
      fraction.js: 4.2.0
      normalize-range: 0.1.2
      picocolors: 1.0.0
      postcss: 8.4.24
      postcss-value-parser: 4.2.0
    dev: false

After manual update to 10.4.16 in package.json I had :

  /autoprefixer@10.4.16(postcss@8.4.24):
    resolution: {integrity: sha512-7vd3UC6xKp0HLfua5IjZlcXvGAGy7cBAXTg2lyQ/8WpNhd6SiZ8Be+xm3FyBSYJx5GKcpRCzBh7RH4/0dnY+uQ==}
    engines: {node: ^10 || ^12 || >=14}
    hasBin: true
    peerDependencies:
      postcss: ^8.1.0
    dependencies:
      browserslist: 4.22.1
      caniuse-lite: 1.0.30001547
      fraction.js: 4.3.7
      normalize-range: 0.1.2
      picocolors: 1.0.0
      postcss: 8.4.24
      postcss-value-parser: 4.2.0
    dev: false
MarArMar commented 1 year ago

Suggestion : adding a manual update tutorial alternative for people who experience this non wanted side effects

ai commented 1 year ago

It should not change versions. It should just call pnpm update -R caniuse-lite.

Seems like it is a bug with project structure.

Can you investigate it and send PR?

MarArMar commented 1 year ago

I am 99.9% certain I saw this in the console during the update : "pnpm u"

So it called pnpm u, and that did the rest

There is probably a call to pnpm u somewhere

ai commented 1 year ago

It is only a single file it is easy to check what does it call.

Here is a pnpm update step https://github.com/browserslist/update-db/blob/main/index.js#L287

foxaltus commented 8 months ago

I'm also seeing the same issue in my CRA project. Many unrelated dependencies get updated after running either npx update-browserslist-db@latest or pnpm dlx update-browserslist-db@latest (the project uses PNPM): image

EDIT: I also get the same when I pnpm up caniuse-lite directly 😕

foxaltus commented 8 months ago

Ohh OK, looks like it's a known issue with PNPM: https://github.com/pnpm/pnpm/issues/5118

My CRA project doesn't have a direct dependency on caniuse-lite, so it seems that pnpm up caniuse-lite is just as good as running pnpm up...

This might need a disclaimer!

Workaround: add caniuse-lite as a dev dependency with pnpm add -D caniuse-lite. Browserlist will no longer complain that the list is outdated and future updates should work.

wesleytodd commented 7 months ago

This dependency is almost certainly a transitive dep in the workflow I just ran and got this message about updating. In nearly all cases printing this is going to be messaging to the wrong people, please consider removing this warning. I will go try and dig around to remove this package from my dependency tree (since I am not sure why it is even in there) if possible, but figured I would drop a message here that printing this message is bad practice even if the @latest was not also incorrect behavior in nearly all cases.

ai commented 7 months ago

This dependency is almost certainly a transitive dep

@wesleytodd do you use Babel or Autoprefixer in your app?

Browserslist prints this dependency only if somebody call browserslist() API. It means that something is using browser data, and it is not just dependency in the lock file. Having old caniuse-lite data means that this browserslist() call will have wrong result.

please consider removing this warning

How do you suggest to force people updating the browser data instead?

Without updating the browser data, all Browserslist config or just default target browsers for Autoprefixer/Babel will contain many outdated versions. It will affect the performance for the whole Web since it will be full of unnecessary polyfills.

For instance, in parallel issue people are complaining that their tool use very old Firefox ESR version. And it is result that they don’t see this warning (they use old tool before we introduced the warning).

Also, could you please be more cooperative in open source and don’t come with negative (“you are all wrong”) mood. The current Browserslist and warning design is the result that I need to support a tool with 50M weekly downloads. And we have many trade-offs. I am always welcome for suggestions, but if a person tries to help and understand all use cases (feel free to ask me, I will explain).

wesleytodd commented 7 months ago

do you use Babel or Autoprefixer in your app

Yep autoprefixer was in my install tree. I have removed it by removing the package I was importing at the top level, that was on it's way out already so it was an easy thing.

How do you suggest to force people updating the browser data instead?

I know nothing about what your package does and so probably cannot say what is the best way. That said, printing on the console is highly disruptive for many reasons. In this case it was printed in some output which was being piped to another command in a cli which expected some particular output. But there are good reasons for libraries not to print to even if it makes sense in the originally designed or intended use case.

The main thing is that I don't directly use the package, and when I talked with my colleague they said they ignore it because it is in all sorts of build output for them. So likely it is not even doing what you hope anyway, which was why I commented here in the first place.

Also, could you please be more cooperative in open source and don’t come with negative (“you are all wrong”) mood. The current Browserslist and warning design is the result that I need to support a tool with 50M weekly downloads. And we have many trade-offs. I am always welcome for suggestions, but if a person tries to help and understand all use cases (feel free to ask me, I will explain).

Sorry if I came across that way. I did not mean to say you are all wrong, but writing to the output of the program in a package like this is not a good practice. I totally understand the need to do things to support large ecosystems (I maintain some large ones, I promise I really do understand), but imagine if a bunch of packages started doing what this one is doing? It would be a mess. I really do mean this feedback in the most positive and helpful ways. Again, sorry if my wording came across strong, I did not intend that but was simply trying to give feedback on this approach.

ai commented 7 months ago

The main thing is that I don't directly use the package, and when I talked with my colleague they said they ignore it because it is in all sorts of build output for them

Why they don’t just fix the problem by calling this command from warning?

In this case it was printed in some output which was being piped to another command in a cli

What is the tool you export data from?

That said, printing on the console is highly disruptive for many reasons.

We are doing for 2 reasons:

  1. We print to stderr, not stdout. It is more popular to print to stderr (like Node.js does it too) warnings.
  2. Autoprefixer/Babel is pretty high-level tools, not just helpers. Most of the users using them or bundlers understand that it is very complex systems to expect some warnings from them.
wesleytodd commented 7 months ago

I will unfollow here. Sorry for the disruption.