[all] Rushstack CI blocked by "browerslist update" warning

[elliot-nelson]

Summary

Looks like recent PR builds are failing due to some warnings (see this build).

--[ WARNING: heft-node-jest-tutorial ]-----------------------[ 8.29 seconds ]--

Browserslist: caniuse-lite is outdated. Please run:
npx browserslist@latest --update-db

Why you should do it regularly:
https://github.com/browserslist/browserslist#browsers-data-updating

--[ WARNING: heft-sass-test ]-------------------------------[ 16.42 seconds ]--

Browserslist: caniuse-lite is outdated. Please run:
npx browserslist@latest --update-db

Why you should do it regularly:
https://github.com/browserslist/browserslist#browsers-data-updating

It's not super clear how to fix this warning, running the suggested command in one of the affected projects doesn't work because there's not a lock file, and running it in the common/temp folder doesn't work because pnpm can't install packages there.

[octogonz]

This has come up a few times before in Zulip chat, for example:

browserslist --update-db

Browserlist Warning

It can be fixed by upgrading indirect dependencies. However fundamentally the browserslist hook is performing a nondeterministic check that will break everyone's ability to build all historical branches of your repo. This is really bad. In the past we recommended for people to avoid browserslist entirely, but that's not always feasible.

Let's use this incident to figure out the right way to disable the browserslist check, and then we can add it to the Rush FAQ.

CC @MickeyPhoenix whose team encountered this also IIRC

[octogonz]

The error is actually coming from this code in the browserslist NPM package:

browserslist/node.js

  oldDataWarning: function oldDataWarning (agentsObj) {
    if (dataTimeChecked) return
    dataTimeChecked = true
    if (process.env.BROWSERSLIST_IGNORE_OLD_DATA) return

    var latest = latestReleaseTime(agentsObj)
    var halfYearAgo = Date.now() - TIME_TO_UPDATE_CANIUSE

    if (latest !== 0 && latest < halfYearAgo) {
      console.warn(
        'Browserslist: caniuse-lite is outdated. Please run:\n' +
        '  npx browserslist@latest --update-db\n' +
        '  Why you should do it regularly: ' +
        'https://github.com/browserslist/browserslist#browsers-data-updating'
      )
    }
  },

[elliot-nelson]

Nice! So there is an env var that could be set.

[octogonz]

The root cause is that they are calling console.warn() instead of console.log(). Someone already pointed that out in issue https://github.com/browserslist/browserslist/issues/361, but they fixed it by introducing this environment variable instead.

The fundamental conflict is that Rush considers console.warn() to be a warning, and warnings breaks CI builds. Having a warning arise due to clock measurements will cause developers to suddenly lose their ability to build old branches, which is a big deal for us.

If console.warn() cannot be eliminated, then perhaps Rush can hack around it by forcing BROWSERSLIST_IGNORE_OLD_DATA=1 for everyone by default. The behavior of Date.now() - TIME_TO_UPDATE_CANIUSE is incompatible with our use case.

@ai what do you think? Would you reconsider eliminating console.warn() in browserlist?

[octogonz]

So there is an env var that could be set.

We could also simply instruct Rush users to set BROWSERSLIST_IGNORE_OLD_DATA=1 themselves. But it's not a great experience. Every repo that encounters this will have to waste time investigating it and rediscovering the solution. And at HBO for example, we'll have to bulk-apply the BROWSERSLIST_IGNORE_OLD_DATA=1 to hundreds of pipeline files, and then make sure every future pipeline remembers to set it. The value of this validation is not worth that headache IMO.

[Faithfinder]

I would prefer if you didn't try to manage third-party libraries for me, that's unintuitive. Borwserslist isn't the only tool that does that, Prisma is similar, and honestly, even PNPM does that, it's just that it's not an issue during rush update (pnpm is .log, not .warn, I guess). I would put a warning in the docs saying "Some tools print update warnings, they usually provide an environment variable to silence it"

[Faithfinder]

Maybe with "here are the ones we know about" and put browserslist and prisma there

[ai]

I have an idea of not showing this warning in case where browsers query doesn't contain time related queries like last 2 versions.

How do you use Browserslist in this project? What is browsers query?

[octogonz]

In this particular repo, we don't use it at all actually. 🙂 It is coming in as an indirect dependency of other NPM packages such as webpack:

https://github.com/microsoft/rushstack/blob/94e2aa7df4a0f9f80a378cdffd67d5c8f3116258/common/config/rush/pnpm-lock.yaml#L17534

[ai]

@octogonz lets me rephrase. Why is Browserslist calling in your stack? Do you compile JS sourced with Babel or use Autoprefixer? (I see these dependencies in lock file, but I am not sure what is this tool about)

[octogonz]

This GitHub repo is a monorepo for a bunch of different build tools, and it also includes some sample projects that are built by the tools. The browserlist package probably gets invoked in multiple different ways.

Looking at the first warning in the log above, heft-node-jest-tutorial is a simple Node.js project that does not use Webpack at all. I can try running it in a debugger and see exactly where browserlist gets loaded. In most cases it's likely to be via an API.

[octogonz]

For this one, looks like Jest is invoking Babel, and then Babel queries browserslist:

The query returns an empty array. The heft-node-jest-tutorial as no browser configuration itself and would never run in a web browser, but apparently Jest is still performing the queries.

[octogonz]

For comparison, here's how HBO's internal monorepo invokes browserslist:

import browserslist from 'browserslist';

function getTargetsFromQuery(query: string[]): string[] {
  try {
    return browserslist(query);
  } catch (error) {
    throw new Error(`Browsers list query for '${query}' failed: ${error.message}`);
  }
}

This is from a specialized tool that builds the 4 standardized polyfill bundles that are shared across all the different app projects and their library projects. This tool has its own JSON input files, and then it generates a query which is passed to browserslist.

So in both cases, a specialized tool is querying the browserslist API; there is no .browserslistrc. (However I'm sure plenty of other Rush consumers do use .browserslistrc in the normal way.)

[ai]

For the case of Jest BROWSERSLIST_IGNORE_OLD_DATA can be safely used. I personally, think that they should pass exact Node.js version to Babel and avoid calling Browserslist for default browsers list, since it reason less (but I am not sure how to fix it).

This outdated caniuse-lite warning is important for building web pages. People can avoid updating caniuse-lite for 2-3 years, which will lead to very old browsers in Babel and Autoprefixer and increase JS/CSS files by unnecessary polyfills and slow down the website.

[octogonz]

This outdated caniuse-lite warning is important for building web pages. People can avoid updating caniuse-lite for 2-3 years, which will lead to very old browsers in Babel and Autoprefixer and increase JS/CSS files by unnecessary polyfills and slow down the website.

Sure, but we should start by agreeing that determinism is an indispensable requirement for professional app development: If two people do git checkout for a given Git hash, and build that code, they should expect to get equivalent output including any errors/warnings, regardless of what day it is. Without this, troubleshooting build failures is a nightmare. (It is the reason why everyone uses package-lock.json or pnpm-lock.yaml for example.)

For the Rush ecosystem, we have a second consideration that console.warn() is interpreted as a warning. And warnings break a CI build. We can't change this.

So if we want to print time-based notices (and I understand why that's important to you), there seem to be basically only two ways to accomplish that:

1. The notices are printed with console.log() and treated as purely informational. This is how PNPM warns about old versions. -OR-
2. The notices are printed with console.warn() but are based on Git-tracked inputs rather than the system clock.

For option 2, here's one possible idea:

• browserslist has an NPM dependency like "browserslist-latest": "*"
• The oldDataWarning() check imports a time value or version number from the browserslist-latest dependency and compares against that
• This way the warning /is/ time based, but its calculation only changes when package-lock.json is updated (because npm install selects a newer version for "browserslist-latest": "*" )

It's just one idea though.

[ai]

Sure, but we should start by agreeing that determinism is an indispensable requirement for professional app development

Absolutely. This is why Browserslist is taken browsers data from the same lock file.

This warning just promote to update caniuse-lite version once every 3-6 months. Between these updates, browsers will be the same between all developers.

We have a problem of falling CI on warnings. It is strange behavior for me since it delete the difference between the error and warning.

The notices are printed with console.log() and treated as purely informational

It will break more system. For instance, you can call a tool with JSON output and this warning will break stdout output.

The notices are printed with console.warn() but are based on Git-tracked inputs rather than the system clock.

It will remove the whole idea of these warnings.

I have another idea. We can think about disabling this warning on process.env.CI since the warning should be readable by user. On CI, nobody sees the warning for most of the cases.

But warning still be shown on non-CI environments.

[octogonz]

We have a problem of falling CI on warnings. It is strange behavior for me since it delete the difference between the error and warning.

Rush's difference is very well-defined:

• Errors are critical issues that prevent downstream tasks/projects from running. For example, if your project does import { X } from 'y' and y does not exist, we can't continue.
• Warnings do NOT prevent downstream tasks/projects from running. They are issues that developers can safely ignore while working (e.g. a lint issue).

Under no circumstances do we allow a PR to be merged with warnings. In a large ecosystem, if you allow that, then your build will quickly accumulate so many "warnings" that everyone ignores them, which defeats the purpose of having warnings. If a person wants to ignore a warning, they must suppress it somehow (e.g. eslint-disable-line) so that other developers don't see the warning in their logs.

The notices are printed with console.warn() but are based on Git-tracked inputs rather than the system clock.

It will remove the whole idea of these warnings.

Why is that? If a branch is under active development, then someone will inevitably need to upgrade an NPM package, and then they will see your warning. If a branch is not under active development, then the warning would only cause trouble.

But warning still be shown on non-CI environments.

This doesn't seem like a great solution. It's still going to cause trouble for developers who pull an old branch and try to build it, and then wonder why Rush is reporting warnings.

[ai]

Why is that? If a branch is under active development, then someone will inevitably need to upgrade an NPM package, and then they will see your warning. If a branch is not under active development, then the warning would only cause trouble.

Even if you are not changing anything in website, it is a good idea to update browsers to reduce JS/CSS files size.

(Also, adding git request will be hard and having git call during Browserslist call will be unexpected for users).

[octogonz]

(Also, adding git request will be hard and having git call during Browserslist call will be unexpected for users).

Maybe you've misunderstood my proposal. As an extremely simplified example, imagine a new NPM package like this:

browserslist-latest/package.json

{
  "name": "browserslist-latest",
  "version": "1.2.3",
  "currentDate": "10/1/2021"
}

Now imagine this dependency relationship:

browserslist-latest/package.json

{
  "name": "browserslist",
  "dependencies": {
    "browserslist-latest": "*"
  }
}

During npm install, the current latest version of the browserslist-latest package will get installed to match "*". So if oldDataWarning() calls require("browserslist-latest/package.json").currentDate the value will be "10/1/2021".

Now suppose that every month we publish a new version of browserslist-latest, for example 1.2.4 has the date "11/1/2021" and 1.2.5 has the date "12/1/2021". In this way, the oldDataWarning() function can still compare against a date, but the date value is 100% deterministic: It only changes when someone upgrades their NPM packages. These upgrades are reasonably frequent for any actively developed project.

(The actual implementation could be different, maybe using a counter or version number instead of currentDate. This is just a thought experiment.)

[ai]

During npm install, the current latest version of the browserslist-latest package will get installed to match "*"

Am I right that npm uses lock file by default? This is why * will be always the same.

In the project where nobody all npm update for years, it will not warn developers of very old polyfills (because of old browsers from Browserslist).

[elliot-nelson]

I think the "*" is actually part of the problem.

Browserslist is not unique, in my mind, in having important updates; if it's a year old that's not great, but you could say that about your version of react or webpack or jest etc as well. None of those packages start warning me if I decide not to update.

What makes browserslist tricky compared to those other dependencies is that I can tell that they are out of date, whereas the "*" dependency makes that harder to tell - it tucks the version away into the lock file instead of making it a normal version update like any other library.

That's the point of the suggestion @octogonz was making -- if you could have a package that was updated once a month or every quarter, with a standard version number, then really you could eliminate both the warning AND the custom update script, because now you are updateable just like a webpack or a typescript.

[ai]

Browserslist is not unique, in my mind, in having important updates; if it's a year old that's not great, but you could say that about your version of react or webpack or jest etc as well. None of those packages start warning me if I decide not to update.

The unique feature of Browserslist is that user explicitly ask for the latest browsers. If developer put last 2 version in .browserslistrc, they expect to have actual versions. Autoprefixer promises to add only actual prefixes (even with no .browserlistrc with default browsers).

There is another important difference: regular updates (without API breaking changes) of Jest, webpack, or React will not (in most of the cases) make your website faster. We have this warning to make Internet in general a better place (at least a little faster).

[elliot-nelson]

@ai Maybe there's two distinct situations then: where the user is intentionally using the features browserslist provides, and where it's a buried dependency they aren't even aware of (like us).

Your earlier suggestion where browserslist won't report the warning unless the query contains certain elements might solve the problem for both: intentional users get notified, and unintentional uses would never see a warning.

[octogonz]

I have an idea of not showing this warning in case where browsers query doesn't contain time related queries like last 2 versions.

Agreed -- maybe this ☝ is the best compromise.

If last 2 versions is a moving target that changes over time (for a given Git hash), then people who care strongly about determinism wouldn't use that setting, and thus won't be impacted by the warning.

And since the Rush Stack and HBO monorepos both are not using .browserslistrc settings (but rather invoking the tool as a library API), then it would solve the problem for us as well.

(It's in fact questionable whether a library API should be printing stuff to console.warn() in the first place -- really it should return the warnings as an array for the caller to handle.)

[ai]

Your earlier suggestion where browserslist won't report the warning unless the query contains certain elements might solve the problem for both: intentional users get notified, and unintentional uses would never see a warning.

I am not sure that it will solve all problems because on no .browserslistrc (and if tool like Jest do not override default value) Browserslist will use default query which uses time-based queries (last 2 versions).

But we can try. I will need a PR for this because I am working on another open source projects right now.

[dmichon-msft]

I would normally expect warnings about package versioning to occur while running package manager operations, e.g. to warn during install, rather than at runtime. In other words "hey, you just installed an ancient version of this package" as opposed to "you're trying to run an ancient version of this package"

[elliot-nelson]

Looking closer at the call stack, it's not obvious to me if there is an easy fix in browserslist. The way I take @ai's suggestion is to change this line to something like this:

  if (/* isNotTimeBased(queries) */) {
    env.oldDataWarning(browserslist.data)
  }

But the implementation of "is not timed based" doesn't seem straightforward. You could specifically check if any of the queries are in the form last 1 year or last 2 versions, but those are only the most obvious. >0.5% is just as time-based (a version used at 2% today won't be in three months). So is "Firefox ESR" (the meaning of ESR changes over time).

We might be able to come up with a list of "allowed queries" that don't trigger a warning (like ["firefox 27"] or ["node 12"] probably don't need warnings). But then the behavior of browserslist itself is unpredictable and confusing to users. It would be better I think if the API of browserslist itself exposed the warning behavior as an option, so it was obvious to callers what the behavior will be:

// Default (warn after 180 days)
browserslist(queries, { oldDataWarningDays: 180 })

// Make warning more aggressive
browserslist(queries, { oldDataWarningDays: 30 })

// Disable warning
browserslist(queries, { oldDataWarningDays: 0 })

This change would keep the current desired behavior, but we could find and propose changes to internal or external tooling that is using browserslist that shouldn't generate warnings.

[elliot-nelson]

There may be a separate ticket to consider here, to make browserslist (caniuse) easier to update in a monorepo.

For the rushstack project, it's obvious that we would never want an outdated caniuse dependency to block CI. But that doesn't mean it's a given for every Rush monorepo maintainer: if all you do is push out large websites, maybe you DO want that warning, and your response to it is to immediately update caniuse.

But, it might be unclear to them how to do so, since the command suggested by the warning doesn't work in a rush monorepo.

[ai]

Having fresh caniuse-lite is the best option. Warning is trying to help here. It is not a false alarm.

[D4N14L]

I do lean toward what @dmichon-msft said here regarding when/where the warning appears. IMO since this is in relation to a package being out of date, it would make the most sense to perform this check during the install as a post-install script. Given that the check is still based on how old the agent itself is (which is provided during install from caniuse-lite), as well as the fact that modifying the install is the only way to mitigate the issue itself, it seems to make sense to place here (especially since it's the only non-runtime place where this type of check can be performed, AFAIK?). We also see a similar pattern for other packages that have issues with dependency versions (ex. node-gyp and Python versions).

Comparing against the date is sub-optimal because of the lack of determinism, but also because it assumes that the package will be maintained and updated for the rest of time forever.... but that does seem to be the general point of the package, and maybe that's fine in this case.

[octogonz]

+1

If the check is performed by a package postinstall hook, that changes things so that the console.warn() message would happen during rush install, which won't get broken by STDERR output. From what I remember, Rush only cares about STDERR output for other commands such as rush build. Installation is treated as a special case.

@ai are you good with this proposal? If so we can probably find someone to make the PR.

[ai]

are you good with this proposal? If so we can probably find someone to make the PR.

It doesn’t solve the problem, which we try to fix by this warning.

The problem of projects with very old dependencies (and caniuse-lite dependency especially).

In these projects, people can have no yarn install run for months. Also, yarn install/npm install output is so long that they will not see the waning.

You are trying to make the warning less visible, ignoring that this warning is really showing the problem. You should give simple tools to update caniuse-lite and promote dependency update routine. Not to hide the warning (by ignoring the problem).

It is cool that CI was broken because it showed the problem in project maintaining.

[Faithfinder]

Just noticed - if #2859 gets released, we can use https://github.com/pnpm/pnpm/issues/4122 to do the upgrades now

[thatchej]

A little bit late to the party here, but is there a relatively painless way to solve this problem out there right now?

I would prefer not to go through and update all of my dependencies in the hope that the transitive version of browserslist would then be correct.

Hoping somebody has a good short term solution!

[elliot-nelson]

If you are getting the warning and need to immediately stop getting the warning, add the following environment variable to your GitHub Action / Azure DevOps / Jenkins build:

BROWSERSLIST_IGNORE_OLD_DATA=1

If you aren't trying to suppress the warning, but rather want to actually upgrade browserslist, that gets trickier.

One approach to force-upgrading if it is deeply nested would be to override in your common/config/rush/.pnpmfile.cjs -- for any package depending on browserslist, set its version to the one you want loaded.

[ryeballar]

You can set the browserslist version manually inside common/config/rush/common-versions.json, under preferredVersions. e.g.

{
  "preferredVersions": {
        "browserslist": "^4.20.4"
  }
}

and then run rush update.

[octogonz]

preferredVersions does not really address the original problem, which was about deterministic builds: If a specific Git branch+commit builds successfully, it should ALWAYS build successfully. Our CI system treats warnings as errors. So we cannot have a tool the prints build warnings one day, when the exact same input did NOT print warnings on a different day.

As seen in this thread, some people don't consider determinism to be important. (We understand this -- it's not necessary to reiterate!🙂👍) Nonetheless determinism /is/ an important requirement for large commercial monorepos.

Setting BROWSERSLIST_IGNORE_OLD_DATA=1 fixes the problem in CI, but not for local development. It is still a nuisance. I was chatting with @elliot-nelson about this again recently, and we're thinking that Rush should offer a config file setting that can force BROWSERSLIST_IGNORE_OLD_DATA=1 for every user and every invocation of rush and rushx command in the monorepo. This will guarantee that nobody encounters these nondeterministic warnings when preparing a PR.

(We do care about the goal of having up-to-date browser data. But if the solution undermines the goal of deterministic builds, it's not an acceptable solution for our use case. From the discussion above, a good compromise would be for the browserslist NPM package to include a postinstall hook that prints the warning during installation only if BROWSERSLIST_IGNORE_OLD_DATA=1. We don't control that part of the solution however; it is up to @ai.)

[ai]

the goal of deterministic builds

I can recommend lock browsers version by name-version in the Browserslist config for deterministic result:

npx browserslist "defaults" > .browserslistrc

We can avoid “outdated caniuse-lite” warning for the configs without time-specific queries (like last 2 versions, >1%).

[Faithfinder]

With rush 5.79 allowing the pnpm.overrides setting, you can use the overrides to upgrade caniuse-lite to the latest version. I think you can even put in latest as the version to upgrade it automatically with every rush update

[octogonz]

With rush 5.79 allowing the pnpm.overrides setting, you can use the overrides to upgrade caniuse-lite to the latest version. I think you can even put in latest as the version to upgrade it automatically with every rush update

Hmmm... specifying * would be okay because it is stable spec and allows the package manager to upgrade when reconsidering those dependencies (e.g. rush update --full). Specifying latest seems to force the package manager to always update the version, which might invalidate some caching assumptions, and could have some counterintuitive effects. Keep in mind that caniuse-lite alters the generation of web bundles, for example Babel transpilation and Autoprefixer CSS generation. (Hypothetically, imagine you are investigating a bug, and you try upgrading one single NPM package to a newer PATCH release, and suddenly 20 different apps in your monorepo pick up an unrelated regression because caniuse-lite got a major upgrade that disabled a transpiler rule.) But maybe I'm being overly paranoid, which does happen after investigating too many arcane bugs. 😁 Or maybe I'm misunderstanding the semantics of latest.

Whether or not we adopt latest, from reading back over this thread, it seems like we still need to do two things:

• Rush can offer a setting of some kind that forces BROWSERSLIST_IGNORE_OLD_DATA=1 for rush and rushx commands, both in CI and for local development. This will disable the nondeterministic logic that it seems @ai prefers not to improve.
• We could also try to implement some kind of analysis during rush update (or even rush install) that prints a warning when caniuse-lite is outdated. This is consistent with other important NPM packages that also print deprecation warnings during this event.

The caniuse-lite warning is ubiquitous, and we hear about it regularly enough that I think it's worth providing a specific solution. The two Rush features could be like:

• Ability to force environment variables to be set in specific contexts (rush install, other rush commands, rushx)
• Ability to warn about outdated versions of specific packages during rush update or rush install