Build: Update nx to 22 and invalidate cache when scripts change #33059
Conversation
📝 WalkthroughWalkthroughUpdated Nx workspace configuration: removed Changes
Sequence Diagram(s)sequenceDiagram
participant Dev/CI
participant Nx
participant Vitest
Note over Nx: test target changes (command + inputs)
Dev/CI->>Nx: run target test (projectName)
activate Nx
Nx->>Vitest: execute "NODE_OPTIONS=--max_old_space_size=4096 yarn vitest run --project={projectName}"
Note right of Vitest: uses inputs "{workspaceRoot}/{vitest*.ts}"\nand other named inputs
Vitest-->>Nx: result (pass/fail)
Nx-->>Dev/CI: exit status
deactivate Nx
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Comment |
|
View your CI Pipeline Execution ↗ for commit 9aa32ff
☁️ Nx Cloud last updated this comment at |
kasperpeulen
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/nx.yml(1 hunks)code/nx.json(3 hunks)code/package.json(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- code/nx.json
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-10-02T09:22:13.215Z
Learnt from: JReinhold
Repo: storybookjs/storybook PR: 32607
File: code/package.json:243-243
Timestamp: 2025-10-02T09:22:13.215Z
Learning: The Storybook repository uses Yarn v^4 (any 4.x version) as the package manager, configured via .yarnrc.yml and package.json packageManager field. Specific patch versions within v4 can be upgraded as needed.
Applied to files:
code/package.json
📚 Learning: 2025-09-17T07:31:04.432Z
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 32484
File: code/core/package.json:326-326
Timestamp: 2025-09-17T07:31:04.432Z
Learning: In Storybook's core package, dependencies like `open` are bundled into the final distribution during the build process, so they should remain in devDependencies rather than being moved to dependencies. End users don't need these packages as separate runtime dependencies since they're included in the bundled code.
Applied to files:
code/package.json
📚 Learning: 2025-09-29T13:20:23.346Z
Learnt from: mrginglymus
Repo: storybookjs/storybook PR: 32556
File: code/core/package.json:309-313
Timestamp: 2025-09-29T13:20:23.346Z
Learning: The `fast-printf` dependency in Storybook's core package is bundled into the final distribution during the build process, so it should remain in devDependencies rather than being moved to dependencies, following the same pattern as other bundled dependencies like `open`.
Applied to files:
code/package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
- GitHub Check: nx
🔇 Additional comments (5)
code/package.json (2)
253-253: VerifyincludedScripts: ["test"]aligns with Nx configuration.The root
testscript (line 54) is now exposed to Nx. Confirm this integrates correctly with any Nx test target configuration updates made incode/nx.json.
110-111: No action required — Nx 22.0.3 upgrade is safe.Verification confirms Nx 22.0.3 is a stable patch release (Nov 10, 2025) with bug fixes. The codebase does not use any breaking-change features: no
createNodesv1, no release config, no deprecated options. @nx/vite v22.0.3 is compatible with Nx 22, which is actively configured in nx.json. Runnx migrateto apply any automated migrations if needed..github/workflows/nx.yml (3)
40-42: Workflow runs only build target; verify test execution is not required here.The PR summary and
ai_summaryindicate thatcode/nx.jsonwas updated with test target configuration changes (command and inputs modified), but this workflow only executesnx affected -t build -c production. Confirm whether tests should also run as part of this CI workflow or if a separate test workflow handles that.
3-7: Workflow triggers only onmainbranch; verify broader trigger scope is not needed.This workflow only executes on pushes to
mainand pull requests, but not on other feature branches. If validation is needed across all branches, consider adjusting the trigger configuration.
19-19: I need to verify iftree:0is a valid Git partial-clone filter syntax.No issues found with
filter: tree:0parameter.The
filter: tree:0parameter is a valid Git partial-clone filter that performs a treeless clone, downloading all reachable commits, then downloading trees and blobs on demand. It requires Git 2.20+ on both server and client. This parameter is documented in GitHub Actions checkout@v4 usage. The syntax in your workflow is correct.
| "@nx/vite": "22.0.3", | ||
| "@nx/workspace": "22.0.3", |
There was a problem hiding this comment.
🧩 Analysis chain
Nx tooling packages placed in dependencies rather than devDependencies.
Build tooling like Nx is typically placed in devDependencies to avoid including it in production distributions. Verify this placement is intentional for the root workspace context.
Also applies to: 196-196
🏁 Script executed:
#!/bin/bash
# Check how other Storybook monorepo roots handle Nx package placement
fd -e 'package.json' -x grep -l '"nx":' {} \; | head -5 | xargs -I {} sh -c 'echo "=== {} ===" && grep -A2 -B2 '"'"'"nx"'"'"' {} | grep -E "(dependencies|devDependencies)" || true'Length of output: 0
I'll investigate the actual placement and context of these Nx packages. Let me examine the root package.json directly and understand the monorepo structure.
---
🏁 Script executed:
```shell
#!/bin/bash
# View the root package.json to see actual Nx package placement
cat code/package.json | jq '.dependencies, .devDependencies' 2>/dev/null || cat code/package.json | grep -A 20 '"dependencies"' | head -40
Length of output: 4234
🏁 Script executed:
#!/bin/bash
# Check if this is indeed the root workspace and look for Nx configuration
ls -la code/ | grep -E '(nx\.json|workspace\.json|package\.json)' && echo "---" && head -30 code/package.jsonLength of output: 981
🌐 Web query:
Nx monorepo root package.json dependencies vs devDependencies best practice
💡 Result:
Short answer — follow this rule of thumb:
- Put only tooling and workspace-level build/test/lint CLI packages in the root package.json as devDependencies (e.g., nx, typescript, eslint, jest, prettier). These are workspace/dev tooling; keep them in devDependencies. [1][2]
- Put runtime (production) dependencies in the package that actually needs them (each library/app package.json → dependencies). Packages that will be published must list their own dependencies. [1][3]
- Avoid putting production dependencies in the root (the root package.json is usually not published; putting prod deps there confuses ownership and publishing). If you must share a runtime dep across many packages, prefer adding it to each consumer’s package.json (or use workspace: / pnpm hoisting patterns deliberately). [2][3]
- For CLIs used by child packages: if the child package’s scripts call the CLI directly, that CLI often needs to be declared in that child package (or referenced with workspace:), otherwise some package managers/CI runners may not expose it. [3][4]
Why: Nx and common monorepo guidance treat the root as the place for toolchain/dev tooling; per Nx docs the workspace installs and links local packages, while publishable/runtime deps must live on the package that’s published. Also linters/test runners work best when colocated as devDependencies at the root. [1][3][4]
If you want, paste your repo layout and package.json snippets and I’ll point out where each dependency should go.
Sources: Nx docs and guides, monorepo/pnpm best-practices. [1] Nx docs (workspace/TypeScript linking & implicit deps). [2] manypkg/monorepo guidance (root should be devDependencies). [3] pnpm/Nx monorepo articles (shared vs package deps). [4] Nx addDependenciesToPackageJson API docs.
I need to check line 196 to see what other issue is flagged. Let me view that context:
---
🏁 Script executed:
```shell
#!/bin/bash
# Check what's at line 196 in code/package.json
sed -n '190,202p' code/package.json
Length of output: 413
Move @nx/vite, @nx/workspace, and nx from dependencies to devDependencies.
The root workspace package.json is private and should follow Nx monorepo best practices: workspace tooling packages like Nx belong in devDependencies, not dependencies. This applies to:
- Lines 110–111:
@nx/viteand@nx/workspace - Line 196:
nx
🤖 Prompt for AI Agents
In code/package.json around lines 110–111 and line 196, the packages @nx/vite,
@nx/workspace (lines 110–111) and nx (line 196) are currently listed under
"dependencies" but should be in "devDependencies"; remove those three entries
from the "dependencies" object and add them to the "devDependencies" object with
the exact same version strings, preserving formatting and commas so the JSON
remains valid.
367a02e to
7f6b5c7
Compare
|
@copilot Fix conflicts |
|
@kasperpeulen I've opened a new pull request, #33076, to work on those changes. Once the pull request is ready, I'll request review from you. |
# Conflicts: # code/yarn.lock
| "@chromatic-com/storybook": "^4.1.2", | ||
| "@nx/vite": "20.8.2", | ||
| "@nx/workspace": "20.8.2", | ||
| "@nx/vite": "22.0.3", |
There was a problem hiding this comment.
Is there a reason why these dependencies are pinned?
|
@kasperpeulen I've opened a new pull request, #33077, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@kasperpeulen I've opened a new pull request, #33078, to work on those changes. Once the pull request is ready, I'll request review from you. |
Closes #
What I did
Update nx to 22 and invalidate cache when scripts change
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
Documentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.🦋 Canary release
This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/coreteam here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=<PR_NUMBER>Summary by CodeRabbit