Skip to content

chore(many): Upgrade nx#20740

Open
dschom wants to merge 1 commit into
mainfrom
upgrade-nx
Open

chore(many): Upgrade nx#20740
dschom wants to merge 1 commit into
mainfrom
upgrade-nx

Conversation

@dschom

@dschom dschom commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Because

  • Now that we are node 24 we can upgrade
  • A new version of NX has been released
  • We want to improve caching

This pull request

  • Upgrades NX, Next, Jest, and other support deps
  • Updates deprecated Jest assertions
  • Fixes a couple caching issues
  • Sets robust nx defaults that work well with our project
  • Removes NX commands from package.json 'scripts'. They are no longer permitted by nx.

Issue that this pull request solves

Closes: FXA-14141

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.
  • I have added necessary documentation (if appropriate).
  • I have verified that my changes render correctly in RTL (if appropriate).
  • I have manually reviewed all AI generated code.

How to review (Optional)

  • Key files/areas to focus on:
  • Suggested review order:
  • Risky or complex parts:

Screenshots (Optional)

Please attach the screenshots of the changes made in case of change in user interface.

Other information (Optional)

There are some commands that now just have exit 0 in them. This is because dependencies are now managed by NX. This means you need use NX to run certain common commands. You can still run the 'sub' commands with yarn workspace directly.

For example, yarn workspace fxa-settings build is now nx run fxa-settings:build OR yarn workspace fxa-settings:build-ts && yarn workspace fxa-settings build-css.

@dschom dschom changed the title Upgrade nx chore(many): Upgrade nx Jun 11, 2026
@dschom dschom force-pushed the upgrade-nx branch 9 times, most recently from 2b8953a to b7e896c Compare June 26, 2026 22:24
@dschom dschom marked this pull request as ready for review June 26, 2026 23:56
@dschom dschom requested a review from a team as a code owner June 26, 2026 23:56

@vbudhram vbudhram left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dschom I tested this locally and it runs fine. There does not seem to be a big increase in start up speed? Just the couple questions otw probably good to merge.

###########################################################
#  ✅ Services ready in 123s (infrastructure was started earlier)
###########################################################

  📍 Verifying started services:
     fxa-content-server   http://localhost:3030  OK
     fxa-profile-server   http://localhost:1111/__heartbeat__  OK
     fxa-auth-server      http://localhost:9000/__heartbeat__  OK
     fxa-settings         http://localhost:3000/settings/static/js/bundle.js  OK
     123done              http://localhost:8080  OK

  All started services responded.
  💡 Run 'yarn ports' to see all service ports
###########################################################

Comment thread migrations.json Outdated
@@ -0,0 +1,105 @@
{

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a stray file?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was auto-generated by nx. But I don't think we need it. I'll remove it.

Comment thread packages/fxa-auth-server/package.json Outdated
]
},
"build-fast": {
"dependsOn": ["prebuild", "^build", "build-l10n", "build-css", "build-copy-assets"]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

build-copy-assets depends on build-ts so it I think it would lose the fast part of reading from source?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. I'll clean this up.

Comment thread packages/fxa-settings/package.json Outdated
},
"build-static": {
"cache": true,
"dependsOn": ["prebuild"],

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was called out in AI review, build-static should depend on build-l10n otherwise it could not bundle the ftl files.

@nshirley

nshirley commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Leaving a comment here from a slack thread, perhaps this fix can be rolled into this PR:

The Start services for playwright tests step jumped from ~2m to ~6.5m because start-services.sh uses -t build --skip-nx-cache. That flag was added in FXA-14006 after #20688 dropped build from the start target. A plain cache hit left content-server's gitignored app/dist bundle missing, so it hung at boot. To restore fast cached builds we would need to declare app/dist as an nx build output for content-server (it's gitignored + not in the defaults), then drop --skip-nx-cache from the start-services.sh.

But, maybe there's a better way to handle this? The short version is just, content-server fails to boot because of an incomplete cache, forcing a full rebuild of the stack for functional-tests.

@dschom dschom force-pushed the upgrade-nx branch 3 times, most recently from 6b22278 to fa411da Compare July 10, 2026 21:53
Because:
- Now that we are node 24, we can use the latest version of NX.
- There are supposed perf benefits

This Commit:
- Upgrades NX, Next, Jest, and other support deps
- Updates deprecated Jest assertions
- Fixes a couple caching issues
- Sets robust nx defaults that work well with our project
- Removes NX commands from package.json 'scripts'. They are no longer permitted by nx.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants