microsoft
[all] Rushstack CI blocked by "browerslist update" warning
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.
This has come up a few times before in Zulip chat, for example:
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
The error is actually coming from this code in the browserslist NPM package:
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'
)
}
},
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?
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.
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 updatepnpm 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"
Maybe with "here are the ones we know about" and put browserslist and prisma there
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?
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
@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)
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.
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.
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.)
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.
This
outdated caniuse-litewarning is important for building web pages. People can avoid updatingcaniuse-litefor 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:
- The notices are printed with
console.log()and treated as purely informational. This is how PNPM warns about old versions. -OR- - 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:
-
browserslisthas an NPM dependency like"browserslist-latest": "*" - The
oldDataWarning()check imports a time value or version number from thebrowserslist-latestdependency and compares against that - This way the warning /is/ time based, but its calculation only changes when
package-lock.jsonis updated (becausenpm installselects a newer version for"browserslist-latest": "*")
It's just one idea though.
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.
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'andydoes 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.
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).
(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.)
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).
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.
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).
@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.
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.)
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.
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"
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.
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.
Having fresh caniuse-lite is the best option. Warning is trying to help here. It is not a false alarm.
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.
