Skip to content

perf(admin-panel): cache the Nx build sub-targets - #21437

Merged
vbudhram merged 1 commit into
mainfrom
fxa-14727
Oct 12, 2026
Merged

vbudhram merged 1 commit into
mainfrom
fxa-14727

Conversation

@fxa-agent

@fxa-agent fxa-agent Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Because

  • Commit 9625ff7 (the Nx upgrade) moved build-css, build-react, build-ts-client and build-ts-server into dependsOn of build, with no cache settings.
  • Nx therefore runs all four on every build. This happens in the CI Build job and in start-services.sh on each Playwright node.

This pull request

  • Adds cache: true, inputs and outputs to the four sub-targets in the nx.targets block of packages/fxa-admin-panel/package.json. build-css is a new entry.

Issue that this pull request solves

Closes: FXA-14727

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: the nx.targets block of packages/fxa-admin-panel/package.json. The scripts block and nx.json are unchanged.
  • Suggested review order: build-css, build-react, then the two build-ts-* targets.
  • Risky or complex parts: the outputs. build/ is not declared as a whole, because webpack cleans build/ but keeps libs/ and packages/ (the server tsc output). build-react declares index.html, favicon.ico and static. build-css and build-react use NODE_ENV as an input, because tailwind.config.js branches on it.
  • One reviewer call: two limits of the prescribed inputs are kept as they are. (a) The production input excludes test and story files. A change to only those files is a cache hit on build-ts-client and skips its type check. (b) build-react lists the two public/ files by name. A new file in public/ must also be added to its outputs. Do you accept both?

Screenshots (Optional)

Other information (Optional)

This change has no test files. This is what I ran in the VM with /fxa-verify --plan. All checks passed.

  • nx show project fxa-admin-panel --json shows cache: true and the inputs and outputs above on all four targets.
  • A second NODE_ENV=test nx run fxa-admin-panel:build took all four sub-targets from cache (15 s).
  • After rm -rf build src/styles/tailwind.out.css, a cached build restored build/index.html, build/favicon.ico, build/static, build/libs, build/packages and tailwind.out.css (16 s).
  • Byte compare: I took the sha256 of every file under build/ plus tailwind.out.css. I compared a cache-restored build with a fresh --skip-nx-cache run of each sub-target. Both lists have 65 files and are identical. For example build/index.html starts with 16f8fc1c7099 and build/favicon.ico with 05f3b705513a.
  • NODE_ENV from test to development, with a fresh isolated Nx cache: build-css and build-react ran. build-ts-client and build-ts-server came from cache.
  • Serve check: the admin-panel server on port 8091 with no PROXY_STATIC_RESOURCES_FROM. Output: GET / 200, index.html assets match build/, GET /static/css/main.1cfb0b2e.css 200, GET /static/js/main.cad72f5a.js 200.
  • packages/functional-tests/tests/admin/adminPanel.spec.ts against that server: 3 passed ("admin panel loads and renders navigation", "admin panel heartbeat is healthy", "admin server heartbeat is healthy").

Not run in the VM, left to CI:

  • The two account search tests in adminPanel.spec.ts. In the VM the API test got 500 because the admin server cannot reach Stripe. The UI test got 403 because a production bundle sends no test-user headers (getExtraHeaders in src/lib/config.ts sends them only when NODE_ENV is development). The byte compare shows the cache does not change the bundle.
  • The CI Build and start-services timings. The baseline is CI job 714508: 81 s for the Build step and 38 s for the start-services build. The CI run of this PR gives the new numbers.

admin-adminPanel-Admin-Pan-8aff4-oads-and-renders-navigation-local.webm

Because:

- The Nx upgrade moved the four fxa-admin-panel build steps into
  dependsOn targets with no cache settings, so Nx reran them on
  every build in CI and in start-services.sh.

This commit:

- Adds cache, inputs and outputs to build-css, build-react,
  build-ts-client and build-ts-server in the nx.targets block.

Closes FXA-14727

Copilot AI 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.

🟡 Changes recommended

Moderate cache correctness issues remain unresolved.

1 open finding
What changed in this PR

Adds Nx caching for the admin-panel CSS, React, and TypeScript build targets to reduce repeated CI and Playwright builds.

Changes:

  • Configures caching inputs and outputs for four build sub-targets.
  • Tracks generated CSS, React assets, and TypeScript outputs.
  • Adds the CSS dependency to the React build target.
File Summary
packages/​fxa-admin-panel/​package.json Adds caching configuration. Findings: two moderate issues regarding TypeScript test/story inputs and incomplete public/ outputs, plus one nit regarding redundant NODE_ENV input.

🧠 Review effort: Lite


💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +168 to +169
"production",
"^production"

@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.

Improvement that only impacts admin panel

@vbudhram
vbudhram merged commit d3c3677 into main Oct 12, 2026
20 of 22 checks passed
@vbudhram
vbudhram deleted the fxa-14727 branch October 12, 2026 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants