task: Bulk-update dependencies by toufali · Pull Request #18768 · mozilla/fxa

toufali · 2025-04-24T21:21:44Z

Because

our dependencies are significantly outdated causing many vulnerability alerts
we see somewhat consistent errors building/deploying image:

➤ YN0000: │ dtrace-provider@npm:0.8.8 STDERR gyp ERR! node -v v22.14.0
➤ YN0000: │ dtrace-provider@npm:0.8.8 STDERR gyp ERR! node-gyp -v v6.1.0
➤ YN0000: │ dtrace-provider@npm:0.8.8 STDERR gyp ERR! This is a bug in `node-gyp`.
➤ YN0000: │ dtrace-provider@npm:0.8.8 STDERR gyp ERR! Try to update node-gyp and file an Issue if it does not help:

This pull request

reinstalls yarn.lock, reevaluating all deps and upgrading where possible
- forces an internal bump of node-gyp from 6.1 to 11.2. This is expected to resolve the build/deploy errors above.
- bumps many additional packages thanks to reevaluated upgrades by yarn. This is expected to make many packages more current, hopefully resolving vulnerabilities and flaky fails as shown above.
- pins or fixes resulting dependency issues. When a pin doesn't fix broken tests (e.g. a result of internal upgrade with breaking change), this PR fixes the issue by bolstering the test or making the code more compliant.

Issue that this pull request solves

Closes: FXA-11513
Closes: FXA-11596

Checklist

Put an x in the boxes that apply

My commit is GPG signed.
If applicable, I have modified or added tests which pass locally.

Other information (Optional)

Multiple dependencies specify node-gyp as an internal dependency with version "latest". At the time of their initial installation, Yarn resolved "latest" to 6.1.0 and recorded this in yarn.lock. Reinstalling yarn.lock requires all deps to be reevaluated based on their current versions.

It's possible that doing this may also resolve other issues, particularly vulnerabilities: It may have been a while since internal deps were updated with a refreshed yarn.lock. I can think of a few vulns that have not been resolved despite us updating them directly, due to old internal deps.

It's also possible that even though yarn should choose the most appropriate deps, we may see internal upgrades that cause breakage...

dschom · 2025-04-28T19:44:48Z

    "@types/convict": "^5.2.2",
    "@types/graphql": "^14.5.0",
-    "@types/jest": "^29.2.5",
+    "@types/jest": "29.2.5",


Can we push these down to the root package.json so it's consistent across all packages/libs?

I think payments-server is stuck on Jest 27 until they eject create-react-app (or more likely) replace with the in-progress SubPlat 3 work.

Thanks for the feedback! Yes, will do on the types. I had already moved typescript down to root 👍

@dschom I actually ended up reverting the typescript packages down to root – surprisingly it was causing a bunch of errors, despite all versions being the same. Guessing something "NX-y" maybe?

Anyway, I agree and want to move all types and similar packages to root, but given how difficult and flaky this initial task has been, I'd prefer to do this properly under separate PR. Unless you disagree, I'll create a new ticket for this?

Sounds good to me!

dschom · 2025-05-05T21:55:11Z


-export default new Random();
+const randomInstance = new Random();
+export default randomInstance;


dschom

Looks good to me. Thanks for all the updates. Remember to squash your commits into a single commit before merging!

chenba · 2025-05-06T13:12:45Z

  },
  "devDependencies": {
-    "@babel/core": "^7.26.0",
+    "@babel/core": "7.25.7",


Downgrading?

I think it's best to keep package groups aligned to the same version, especially if we intend to eventually move these down to root to make future maintenance easier.

The @babel group had a range of versions between 7.16 and 7.26. I couldn't find any @babel/core resolutions in the 7.26 range, so I chose 7.25.7 which seemed the most widely used in the group and still relatively recent (released Oct).

I'm happy to try to bump everything up to 7.26 – it might "just work"! It's also possible some adjacent packages may not go that high, or issues arise (which we could try to resolve via code/test fixes). I time-boxed myself on this PR since it ballooned from the original ask, but I'm happy to keep going on it!

I'm happy to try to bump everything up to 7.26

No, or at least I don't know enough about the babel packages to say. But keeping them in lock-step sounds good.

chenba · 2025-05-06T13:22:15Z

-        style: 'normal',
-        weight: 'semibold',
+        fontStyle: 'normal',
+        fontWeight: 'semibold',


cc @vpomerleau

I'm not sure the original code ever worked? 😅

Looking at the docs, they specify fontStyle and fontWeight. Even earlier versions of the react-pdf package suggest those are the correct keys. I tried pinning related packages but the error persisted until I used the keys as suggested in the docs...

Thanks for fixing! The previous package versions must have been validating more loosely and this was missed. FWIW, I tested before and after this change, the PDF result is the same. fontStyle and fontWeight might be unnecessary here.

Thanks for testing @vpomerleau !

chenba · 2025-05-06T13:23:18Z

    ).toBeInTheDocument();
    const disabledSubmitButton = screen.getByRole('button', {
-      name: /Enter 6-digit code to continue/i,
+      name: /Confirm/i,


Doesn't seem to be dep upgrade related.

After the dep upgrades it seemed some tests became "more aware" 😅 I'm not sure why the tests weren't failing before, but I checked these tests and the matching name indeed had been updated to "Confirm".

Well that's... concerning.

chenba · 2025-05-06T13:24:11Z

-    await userEvent.click(screen.getByRole('link', { name: 'Help' }));
+    await userEvent.click(
+      screen.getByRole('link', { name: 'Help Opens in new window' })
+    );


Doesn't seem to be dep upgrade related?

As above, certain upgraded packages seemed to make tests "more aware" – in this case it was trying to match 'Help', but there was an extra <span> for screenreaders that was added ("Opens in new window") and now included in the match.

dschom · 2025-05-07T00:43:34Z

    "@types/hapi__hapi": "^20.0.10",
    "@types/ioredis": "^4.26.4",
-    "@types/jsonwebtoken": "^8.5.1",
+    "@types/jsonwebtoken": "8.5.1",


Not anything you did, but I find it odd that the types are now set to 8.5.1 and the actual package jsonwebtoken is now referencing 9.0.0... Seems like this could cause some bad types...

I'd like to think the @types/* and the corresponding package should be the same.

Yes, I agree – we can try to bump the types to match the version. FWIW, this currently resolves as if pinned at 8.5.1 anyway on main:

├─ fxa-auth-server@workspace:packages/fxa-auth-server │ └─ @types/jsonwebtoken@npm:8.5.1 (via npm:^8.5.1)

That's because of the semversion directive, ^. That'll stop it from from getting bumped to 9.X, and it'll use the highest 8.X version available, which is 8.5.1. Using the caret, is the idiomatic way to install packages, and I don't think we should remove the caret here.

dschom · 2025-05-07T01:02:43Z

    "joi": "^17.13.3",
    "jose": "^5.9.6",
-    "jsonwebtoken": "^9.0.0",
+    "jsonwebtoken": "9.0.0",


On the first pass of this PR, I took it at face value that these dependencies were all somehow impacted by node-gyp, but I thought I'd check that assumption and am now a bit confused. If look at the deps for jsonwebtoken (and quite a few other pinned versions here), I don't see node-gyp as downstream dependency, e.g. https://npmgraph.js.org/?q=jsonwebtoken .

So I guess I'm wondering why this particular dependency needed to be pinned? It's rather unconventional to do this. The issue is that we won't get minor updates or patches, both of which could contain security / bug fixes and in theory shouldn't contain breaking changes.

Is this just the result of trail and error after deleting the yarn lock file?

Let me give a bit more context! Sorry if some of this is known info:

node-gyp is not a direct dependency of ours, it's a dep of multiple deps. Yarn.lock had it stuck at version 6.1. We could have added another resolution to try forcing a later version, but this is usually considered last resort. More ideal would be to keep up with dependency maintenance. I think the large, scattered number of deps we have makes this hard.

I don't think we get automatic minor updates or patches as is? Any time yarn install is run, it simply resolves to what's in the lock file – which is what got us to the node-gyp issue in the first place. Another case-in-point, I recently tried resolving an Axios vulnerability, but because the vulnerable version wasn't a direct dependency, the vuln persists. It seemed like many packages could benefit from an upgrade – I think they've been locked for a long time?

Conversely, upgrading our lock file did auto-bump minor updates/patches (maybe this is what you meant). Some of these upgraded packages broke our builds/tests. Where possible, I pinned these to their previously resolved version. So from your example, the jsonwebtoken dep of auth-server on main resolves (and is locked) to version 9.0 despite the caret:

fxa-auth-server@workspace:packages/fxa-auth-server │ └─ jsonwebtoken@npm:9.0.0 (via npm:^9.0.0)

My thinking was this yarn lock upgrade could be a first pass to catch up with our dependency maintenance, and anything that we pin here we can investigate for future package maintenance, and resolve the breakage one-by-one (and unpin when safe to do so).

Sorry for the length of this explanation! Let's chat more in realtime if I've missed anything or you still have concerns.

Thanks for the detailed explanation. Makes sense nuking the yarn lock file and reinstalling inevitably broke things and pinning certain versions fixed them.

For this particular dependency I'm dubious that we needed to removed the ^ on json webtoken. Basically, I'm pretty skeptical that a 9.0.2 or 9.0.1 would break our tests but 9.0.0 wouldn't! This is a popular package, and a patch shouldn't break stuff per the semversioning standard, but I've certainly seen crazier things. I'm going to test this myself real quick and see what happens.

I branched from this branch to do a quick experiment. Let's see what happens.

#18830

I totally agree! Honestly, things got hectic as I was trying to pinpoint (pun intended) what caused breakage. For example, I retested unpinning @opentelemetry/semantic-conventions and sure enough it works now. I was still getting a handle on how to test things locally, etc. You're more familiar with these packages and the repo, so I defer to you.

We can always fast-follow these things too, including moving duplicate packages to root. Pinning it at 9 is the same functionality we currently have on main, resolving to 9.

Looks like the tests passed in #18830 with the upgraded jsonwebtoken.

Yep, let's file some fast follows for these. Mass updating packages can be maddening when tests break. I think this PR is great starting point for. Let's revisit these pinned deps individually and get it all cleaned up.

Your experiment worked! Thanks for the sanity check. I just pushed unpinned version 9.0.2 🚀

chenba · 2025-05-07T13:56:11Z

    "grunt-contrib-copy": "^1.0.0",
    "grunt-contrib-watch": "^1.1.0",
    "grunt-http": "^2.3.3",
+    "husky": "^9.1.7",


errant caret?

no – for this case, husky broke bigly and pinning was troublesome.

The current version of husky loads config via distinct pre-commit, post-checkout, and post-merge files. It was easy enough to modernize the package and safe to keep the caret.

chenba

WFM. But let's also wait for Dan's approval.

dschom · 2025-05-07T15:57:13Z

-    "@sentry/nextjs": "^8.42.0",
-    "@sentry/node": "^8.42.0",
-    "@sentry/opentelemetry": "^8.42.0",
+    "@sentry/browser": "8.42.0",


For these package groups, I'm a fan of pinning versions! I've seen things break because these get out sync. 👍

dschom · 2025-05-07T16:00:09Z

    "@svgr/webpack": "^8.0.1",
    "@types/node": "^20.11.1",
-    "@types/react": "^18",
+    "@types/react": "18.2.14",


This should be inline with our react version, which is 18.3.1. Why did 18.2.14 get picked?

18.2.14 is what main resolves to. Instead of going to React's version 18.3.1, it probably bumped all the way to 18.3.12, possibly causing breakage? Seems reasonable that we could pin it to match React though – I will flag this and other pins/resolutions in a separate ticket to follow up on.

dschom

Thanks for all the work here. I'm going to r+. I'm sure it was a tedious process.

I do think there might be some follow up issues to file here. Typically, I don't think pinning versions is necessary... Perhaps these pinned versions can be evaluated on a case by case basis? On the other hand, overtime dependabot will update them anyways... so maybe this doesn't matter so much.

The thing that does give me some pause is the versions in our resolution file. They are mostly band aides and can end up causing problems down the road especially if an exact version is pinned. It'd be great to reduce the number of resolutions or at the very least have a really clear reason of for each resolution documented somewhere.

I've always been a little scared to touch the resolutions, but have also been bitten by a bad resolution that no longer made sense. I feel like our resolutions are outside the scope of this PR, but it is somewhat related to the state of lock file. Revisiting this would be a nice bit of clean up.

chenba · 2025-05-07T16:31:10Z

The thing that does give me some pause is the versions in our resolution file. ...

Agreed. Majority of them were band-aides over security vulns. I think they are overdue for a review. Thanks, @dschom

- upgrade and migrate to husky v9 - add ip undefined fallback to gql-customs to maintain type String - fix keys for settings pdf - fix anonymous-default-export - fix session-token.strategy test - fix integration test sinon ESM Jest transform - add "compression" resolution to fix "content encoding error" in Playwright test - pin stripe, typescript, flat, babel, jsonwebtoken, storybook, sentry, class-validator, and twilio

toufali requested a review from a team as a code owner April 24, 2025 21:21

toufali force-pushed the fix-nightly-ci-node-gyp branch 3 times, most recently from f7a5915 to 4423895 Compare April 24, 2025 23:48

dschom reviewed Apr 28, 2025

View reviewed changes

toufali force-pushed the fix-nightly-ci-node-gyp branch 25 times, most recently from 2e53697 to f30ca77 Compare April 29, 2025 22:29

dschom reviewed May 5, 2025

View reviewed changes

toufali force-pushed the fix-nightly-ci-node-gyp branch from 0df6124 to adeea8f Compare May 5, 2025 23:15

chenba reviewed May 6, 2025

View reviewed changes

Comment thread packages/fxa-graphql-api/src/auth/gql-customs.guard.ts Outdated

chenba reviewed May 6, 2025

View reviewed changes

Comment thread packages/fxa-settings/package.json Outdated

chenba reviewed May 6, 2025

View reviewed changes

toufali force-pushed the fix-nightly-ci-node-gyp branch 2 times, most recently from 25a516f to 2534793 Compare May 6, 2025 22:47

toufali requested a review from chenba May 6, 2025 23:34

dschom reviewed May 7, 2025

View reviewed changes

chenba reviewed May 7, 2025

View reviewed changes

chenba approved these changes May 7, 2025

View reviewed changes

dschom reviewed May 7, 2025

View reviewed changes

Comment thread package.json

dschom approved these changes May 7, 2025

View reviewed changes

toufali force-pushed the fix-nightly-ci-node-gyp branch from 2534793 to 7fbf59d Compare May 7, 2025 16:37

toufali force-pushed the fix-nightly-ci-node-gyp branch from 7fbf59d to bd4ced4 Compare May 7, 2025 16:47

toufali merged commit be9fc34 into main May 7, 2025
21 checks passed

toufali deleted the fix-nightly-ci-node-gyp branch May 7, 2025 17:26

toufali changed the title ~~fix: Nightly CI run fails to create/deploy image~~ task: Bulk-update dependencies May 30, 2025

This was referenced Jun 11, 2025

task: audit yarn resolutions – compression #19012

Merged

task: audit yarn resolutions – flat #19013

Merged

toufali mentioned this pull request Jun 26, 2025

task: audit yarn resolutions – typescript #19107

Merged

Conversation

toufali commented Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Because

This pull request

Issue that this pull request solves

Checklist

Other information (Optional)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

toufali Apr 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dschom left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

toufali May 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

toufali May 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dschom May 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dschom May 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

toufali May 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dschom May 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

toufali commented Apr 24, 2025 •

edited

Loading

toufali Apr 28, 2025 •

edited

Loading

toufali May 6, 2025 •

edited

Loading

toufali May 6, 2025 •

edited

Loading

dschom May 7, 2025 •

edited

Loading

dschom May 7, 2025 •

edited

Loading

toufali May 7, 2025 •

edited

Loading

dschom May 7, 2025 •

edited

Loading

toufali May 7, 2025 •

edited

Loading