Build: No nx cache in circle ci #33230
Conversation
kasperpeulen
changed the title
|
View your CI Pipeline Execution ↗ for commit be47220
☁️ Nx Cloud last updated this comment at |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughEnforces --skip-nx-cache in build task invocations, removes the separate Compile CI step (inlining yarn before check), standardizes many package.json repository URLs to git+https, and expands Verdaccio-based local registry publishing with workspace-aware npm publish flow and related tooling changes. Changes
Sequence Diagram(s)sequenceDiagram
participant DevEnv as Local workspace (scripts/run-registry)
participant Registry as Verdaccio (local)
participant NPM as npm CLI
participant FS as Filesystem (temp dirs)
participant Workspaces as Monorepo packages
note over DevEnv,Registry: Publish flow (new)
DevEnv->>Workspaces: collect workspace package paths & versions
DevEnv->>DevEnv: compute publishVersion (RUN_ID-tagged)
DevEnv->>FS: mkdtemp & cp package -> temp dir
DevEnv->>FS: rewrite package.json (version + resolve workspace: deps)
DevEnv->>FS: write local .npmrc pointing to Registry
DevEnv->>NPM: npm publish (cwd=temp dir) -> Registry
NPM-->>Registry: publish package tarball & metadata
Registry-->>DevEnv: publish success (or error)
DevEnv->>FS: rm temp dir (cleanup)
note over DevEnv,Registry: Concurrency limit applies to multiple packages
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
scripts/tasks/check.ts(1 hunks)scripts/tasks/compile.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use ESLint and Prettier configurations that are enforced in the codebase
Files:
scripts/tasks/check.tsscripts/tasks/compile.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode
Files:
scripts/tasks/check.tsscripts/tasks/compile.ts
🧠 Learnings (1)
📚 Learning: 2025-11-28T14:50:24.872Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.872Z
Learning: Applies to code/**/*.{js,jsx,json,html,ts,tsx,mjs} : Run ESLint with yarn lint:js:cmd to check for linting issues and fix errors before committing
Applied to files:
scripts/tasks/check.ts
⏰ 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). (2)
- GitHub Check: nx
- GitHub Check: Core Unit Tests, windows-latest
| const linkCommand = `yarn nx run-many -t check ${parallel} --skip-nx-cache`; | ||
| const nolinkCommand = `yarn nx run-many -t check -c production ${parallel} --skip-nx-cache`; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Read the check.ts file
wc -l scripts/tasks/check.ts
echo "---"
cat -n scripts/tasks/check.tsRepository: storybookjs/storybook
Length of output: 1286
🏁 Script executed:
#!/bin/bash
# Check git log for recent changes to this file
git log --oneline -n 20 -- scripts/tasks/check.ts
echo "---"
# Check the diff for this file in the current branch
git diff HEAD~1 HEAD -- scripts/tasks/check.ts 2>/dev/null || git show HEAD:scripts/tasks/check.ts 2>/dev/nullRepository: storybookjs/storybook
Length of output: 1079
🏁 Script executed:
#!/bin/bash
# Search for other uses of process.env.CI in the codebase
rg 'process\.env\.CI' scripts/tasks/ -n -C 2
echo "---"
# Check other task files for patterns
ls -la scripts/tasks/*.ts | head -20Repository: storybookjs/storybook
Length of output: 2268
🏁 Script executed:
#!/bin/bash
# Get more details about the recent commit
git show --stat 1c31eaa56d
echo "---"
# Check if there are any related issues or PRs mentioned
git log --all --grep="cache" --oneline | head -10Repository: storybookjs/storybook
Length of output: 50378
Based on my verification of the codebase, I can now provide the rewritten review comment:
Apply --skip-nx-cache only in CI environments to preserve local performance.
The code applies --skip-nx-cache unconditionally in both linkCommand and nolinkCommand, affecting local development and CI equally. However, the commit message "No nx cache in circle ci" indicates CI-only intent.
The codebase already establishes a pattern for environment detection (line 9 uses process.env.CI). Apply the same pattern here to avoid slowing down local type-checking iterations:
+const cacheFlag = process.env.CI ? '--skip-nx-cache' : '';
-const linkCommand = `yarn nx run-many -t check ${parallel} --skip-nx-cache`;
-const nolinkCommand = `yarn nx run-many -t check -c production ${parallel} --skip-nx-cache`;
+const linkCommand = `yarn nx run-many -t check ${parallel} ${cacheFlag}`;
+const nolinkCommand = `yarn nx run-many -t check -c production ${parallel} ${cacheFlag}`;🤖 Prompt for AI Agents
In scripts/tasks/check.ts around lines 11 to 12, the commands unconditionally
append --skip-nx-cache which disables the Nx cache for local runs; change them
to only include --skip-nx-cache when running in CI (use the existing
process.env.CI check pattern from line 9). Build the parallel variable as
currently done, then append --skip-nx-cache to linkCommand and nolinkCommand
only when process.env.CI is truthy so local developers keep cached runs.
| } | ||
| }, | ||
| async run({ codeDir }, { link, dryRun, debug, prod, skipCache }) { | ||
| async run({ codeDir }, { link, dryRun, debug, prod }) { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for skipCache references across the codebase
rg -n -C3 --type=ts 'skipCache' -g '!scripts/tasks/compile.ts'Repository: storybookjs/storybook
Length of output: 5583
🏁 Script executed:
#!/bin/bash
# Check the compile task definition and function signature
echo "=== Task option definitions ==="
rg -n -A 30 'compile:' scripts/task.ts | head -60
echo -e "\n=== Compile.ts function signature ==="
sed -n '35,45p' scripts/tasks/compile.tsRepository: storybookjs/storybook
Length of output: 515
🏁 Script executed:
#!/bin/bash
# Search for compile task option definitions in scripts/task.ts
rg -n 'compile' scripts/task.ts -A 5 -B 5 | head -80
# Also check the actual task.ts structure around task options
echo -e "\n=== Task.ts structure ==="
sed -n '1,50p' scripts/task.tsRepository: storybookjs/storybook
Length of output: 3252
🏁 Script executed:
#!/bin/bash
# Find task options definition
rg -n 'createOptions' scripts/task.ts -A 100 | head -150
# Also check if there's a per-task options definition
echo -e "\n=== Checking compile task definition ==="
cat scripts/tasks/compile.ts | head -50Repository: storybookjs/storybook
Length of output: 6898
🏁 Script executed:
#!/bin/bash
# Check check.ts to see if it has the same issue
echo "=== check.ts function signature ==="
rg -n 'async run' scripts/tasks/check.ts -A 3
# Also verify if skipCache is used by any task
echo -e "\n=== Which tasks use skipCache from options ==="
rg -n 'skipCache' scripts/tasks/*.ts | grep -v compile.ts | grep -v check.tsRepository: storybookjs/storybook
Length of output: 292
Perfect! I now have enough information to provide a comprehensive rewritten review comment. The verification confirms the parameter removal is safe, but also reveals an additional consistency issue.
Verify that the skipCache task option is also removed from scripts/task.ts for consistency.
The parameter removal at line 40 is safe—no calling code passes skipCache to compile.run(). However, skipCache is still defined as a global task option at scripts/task.ts:168–172 but is now ignored by the compile task (and similarly by check at scripts/tasks/check.ts:19). Since cache skipping is now unconditional, remove the skipCache option from the task options to prevent users from passing a parameter that has no effect.
🤖 Prompt for AI Agents
In scripts/tasks/compile.ts around line 40, the run signature removed the
skipCache parameter and now unconditionally skips the cache; update the global
task options in scripts/task.ts (around lines 168–172) to remove the unused
skipCache option so users cannot pass a no-op flag, and likewise remove any
remaining references or docs mentioning skipCache (also check
scripts/tasks/check.ts where skipCache was removed) to keep the API and
documentation consistent; ensure no callers expect skipCache before deleting.
| await rm(join(codeDir, 'bench/esbuild-metafiles'), { recursive: true, force: true }); | ||
| return exec( | ||
| `${command} ${skipCache ? '--skip-nx-cache' : ''}`, | ||
| `${command} --skip-nx-cache`, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's read the compile.ts file to see the context around line 44
head -60 scripts/tasks/compile.ts | tail -30Repository: storybookjs/storybook
Length of output: 927
🏁 Script executed:
#!/bin/bash
# List files in scripts/tasks directory
ls -la scripts/tasks/ | head -20Repository: storybookjs/storybook
Length of output: 1260
🏁 Script executed:
#!/bin/bash
cat -n scripts/tasks/compile.tsRepository: storybookjs/storybook
Length of output: 2349
🏁 Script executed:
#!/bin/bash
cat -n scripts/tasks/check.tsRepository: storybookjs/storybook
Length of output: 1256
🏁 Script executed:
#!/bin/bash
# Check git status and see what was modified
git diff HEAD~1 scripts/tasks/compile.ts scripts/tasks/check.ts 2>/dev/null || git diff scripts/tasks/compile.ts scripts/tasks/check.ts 2>/dev/null || echo "Could not get diff"Repository: storybookjs/storybook
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Check if there's a way to see what branch we're on and what the intent is
git log --oneline -10 2>/dev/null | head -5 || echo "No git log available"
# Also check for environment variables that might indicate CI
env | grep -i "ci\|circle\|github" || echo "No CI env vars found"Repository: storybookjs/storybook
Length of output: 108
Based on my verification, I can now provide the rewritten review comment:
Apply cache skipping only in CI environments to preserve local development performance.
The PR intent is CI-specific per the commit message "No nx cache in circle ci", yet --skip-nx-cache is currently applied unconditionally in all environments. This impacts local build speed. The codebase already uses process.env.CI for environment-specific logic (line 9 in both compile.ts and check.ts for parallelization), so apply the same pattern for cache skipping:
- return exec(
- `${command} --skip-nx-cache`,
- { cwd: ROOT_DIRECTORY },
+ const cacheFlag = process.env.CI ? '--skip-nx-cache' : '';
+ return exec(
+ `${command} ${cacheFlag}`,
+ { cwd: ROOT_DIRECTORY },The same issue exists in check.ts (lines 11-12).
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In scripts/tasks/compile.ts around line 44, the command array unconditionally
includes `--skip-nx-cache`, which slows local development; change it to append
`--skip-nx-cache` only when process.env.CI is truthy (same pattern used for
parallelization at line 9). Also update scripts/tasks/check.ts at lines 11-12 to
conditionally add the `--skip-nx-cache` flag only in CI by checking
process.env.CI. Ensure both places build the command array dynamically so local
runs do not include the skip-cache flag while CI runs do.
There was a problem hiding this comment.
Pull request overview
This PR removes the nx cloud connection in CircleCI to prevent conflicts from having multiple separate connections, which can cause nx to malfunction. The key changes include forcing --skip-nx-cache on compilation and type-checking, simplifying the Verdaccio registry configuration for local testing, refactoring the package publishing process to use unique versioned packages, and enabling Yarn's global cache.
- Disabled nx remote caching by always applying
--skip-nx-cacheflag in compile and check tasks - Refactored local registry publishing to use timestamped versions (e.g.,
version-local.timestamp) to avoid cache conflicts - Simplified Verdaccio configuration to remove authentication and proxy rules for local testing
Reviewed changes
Copilot reviewed 55 out of 56 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
.circleci/src/jobs/check.yml |
Removed separate compile step; check task now handles dependency installation directly |
.circleci/config.yml |
Mirror of the source file changes - removed compile step from check job |
scripts/tasks/compile.ts |
Always applies --skip-nx-cache flag unconditionally during compilation |
scripts/tasks/check.ts |
Always applies --skip-nx-cache flag to both linked and production check commands |
scripts/verdaccio.yaml |
Simplified configuration - removed auth, extensive package proxy rules, and added offline mode |
scripts/run-registry.ts |
Complete refactor to publish uniquely versioned packages with timestamps; removed npm auth dependency |
scripts/utils/yarn.ts |
Re-enabled global cache and removed storybook version resolutions from package.json |
scripts/package.json |
Upgraded verdaccio from optional v5 to required v6 dependency |
code/core/src/bin/dispatcher.ts |
Changed version lookup to use storybook package.json instead of versions map |
code/core/src/common/js-package-manager/*.ts |
Added cwd parameter to package info commands for proper registry resolution |
code/core/src/common/js-package-manager/JsPackageManager.ts |
Added special handling for -local. versioned packages to return local tag |
Multiple package.json files (renderers, frameworks, builders, etc.) |
Standardized repository URLs by adding git+ prefix |
| const publishPackage = async (ws: Package, workspaces: Package[]) => { | ||
| const info = workspaces.find((it) => it.name === ws.name); |
There was a problem hiding this comment.
The function searches for ws.name within workspaces to find info, but ws and workspaces are the same array (they're both of type Package[]). The find will always return the same item. This redundant lookup serves no purpose. If the intention was to reuse the same object reference, you can directly use ws.publishVersion instead of info.publishVersion at line 157.
| resolveWorkspaceDeps(pkg, workspaces); | ||
|
|
||
| await writeFile(join(tmpDir, 'package.json'), JSON.stringify(pkg, null, 2)); | ||
| await writeFile(join(tmpDir, '.npmrc'), `//localhost:6002/:_authToken=fake`); |
There was a problem hiding this comment.
The auth token uses hardcoded port 6002 instead of the VERDACCIO_PORT constant defined at line 23. This creates a maintenance issue if the port needs to be changed. Use //localhost:${VERDACCIO_PORT}/:_authToken=fake for consistency.
| // We don't want to use the cache or we might get older copies of our built packages | ||
| // (with identical versions), as yarn (correctly I guess) assumes the same version hasn't changed | ||
| // TODO publish unique versions instead | ||
| `yarn config set enableGlobalCache false`, | ||
| `yarn config set enableGlobalCache true`, |
There was a problem hiding this comment.
The comment states "We don't want to use the cache or we might get older copies of our built packages", but the code on line 130 has been changed to set enableGlobalCache true. Either the comment needs to be updated to reflect the new caching strategy, or the comment should explain why caching is now acceptable (e.g., because unique versions are being published with timestamps).
| async run({ codeDir }, { link, dryRun, debug, prod }) { | ||
| const command = link && !prod ? linkCommand : noLinkCommand; | ||
| await rm(join(codeDir, 'bench/esbuild-metafiles'), { recursive: true, force: true }); | ||
| return exec( | ||
| `${command} ${skipCache ? '--skip-nx-cache' : ''}`, | ||
| `${command} --skip-nx-cache`, |
There was a problem hiding this comment.
The function signature was changed to remove the skipCache parameter, but the --skip-nx-cache flag is now always applied unconditionally. This means that the skipCache option defined in scripts/task.ts (line 168-172) is no longer used by this function. Consider either:
- Removing the
skipCacheoption from the task options if it's no longer needed - Or documenting that this option is deprecated/unused for the compile task
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
code/lib/csf-plugin/package.json (1)
69-69: Fix invalid git SHA in gitHead field.The
gitHeadvalue ends with a lowercasel(letter L) instead of a hexadecimal digit. Valid git SHA-1 hashes contain only hex characters (0-9, a-f). This appears to be a typo where1(digit one) was mistyped asl(letter L).Apply this diff:
- "gitHead": "a8e7fd8a655c69780bc20b9749d2699e45beae1l" + "gitHead": "a8e7fd8a655c69780bc20b9749d2699e45beae11"code/frameworks/vue3-vite/package.json (1)
69-69: Fix invalid gitHead hash—contains non-hexadecimal character.The gitHead value ends with
l(lowercase letter L), but git commit hashes must contain only hexadecimal characters (0–9, a–f). This is a corruption in the package metadata that can break npm registry integrity verification.Apply this diff to correct the hash:
- "gitHead": "a8e7fd8a655c69780bc20b9749d2699e45beae1l" + "gitHead": "a8e7fd8a655c69780bc20b9749d2699e45beae11"code/frameworks/react-vite/package.json (1)
76-76: Correct the invalid hexadecimal character in gitHead.The
gitHeadvalue ends with"1l"(digit one followed by letter L), which is invalid hexadecimal. Git commit hashes use only 0–9 and a–f. Correct the final character:- "gitHead": "a8e7fd8a655c69780bc20b9749d2699e45beae1l" + "gitHead": "a8e7fd8a655c69780bc20b9749d2699e45beae11"
♻️ Duplicate comments (3)
scripts/utils/yarn.ts (1)
121-140: Comment and caching behavior are inconsistentThe comment still says “We don't want to use the cache…” but the code now sets
enableGlobalCachetotrue. Please either:
- Update the comment to describe why global cache is now safe (e.g., unique/timestamped versions in Verdaccio), or
- Revert
enableGlobalCachetofalseif avoiding the cache is still required.Also worth double‑checking that
enableGlobalCachebehaves as expected with the Yarn 4.9.x setup in this repo. Based on learnings, the project standardizes on Yarn 4.scripts/run-registry.ts (2)
150-151: Redundant lookup detected.The
findon line 151 searches forws.namein the sameworkspacesarray thatwscomes from, which will always return the matching item. You can usews.publishVersiondirectly at line 157 instead ofinfo.publishVersion.
161-161: Inconsistent port usage.The
.npmrcauth token uses hardcoded port6002while line 169 correctly uses theVERDACCIO_PORTconstant. Use template literal for consistency:- await writeFile(join(tmpDir, '.npmrc'), `//localhost:6002/:_authToken=fake`); + await writeFile(join(tmpDir, '.npmrc'), `//localhost:${VERDACCIO_PORT}/:_authToken=fake`);
🧹 Nitpick comments (3)
code/core/src/bin/dispatcher.ts (1)
77-83: Good approach to dynamic version resolution.Importing the version directly from
storybook/package.jsonimproves maintainability and ensures version consistency compared to relying on the hardcodedversionsobject.Consider adding error handling around the
storybook/package.jsonimport (lines 77-79) for defensive programming:+ let packageJson; + try { - const { default: packageJson } = await import(`storybook/package.json`, { + const imported = await import(`storybook/package.json`, { with: { type: 'json' }, }); + packageJson = imported.default; + } catch (error) { + logger.error('Failed to load storybook package.json'); + process.exit(1); + }While this scenario is unlikely (the dispatcher itself is part of the storybook package), explicit error handling prevents unhandled promise rejections.
scripts/run-registry.ts (2)
250-252: Consider graceful handling of individual publish failures.If any single package publish fails,
Promise.allwill reject immediately, potentially leaving some packages unpublished while others succeeded. For local testing this may be acceptable, but consider usingPromise.allSettledif partial success reporting is useful.- await Promise.all(packages.map((p) => limit(() => publishPackage(p, packages)))); + const results = await Promise.allSettled(packages.map((p) => limit(() => publishPackage(p, packages)))); + const failures = results.filter((r) => r.status === 'rejected'); + if (failures.length > 0) { + logger.error(`Failed to publish ${failures.length} packages`); + failures.forEach((f) => logger.error((f as PromiseRejectedResult).reason)); + throw new Error(`Failed to publish ${failures.length} packages`); + }
212-212: UseREGISTRY_PORTconstant for consistency.The URL uses hardcoded
6001instead of theREGISTRY_PORTconstant defined at line 22.- const verdaccioUrl = `http://localhost:6001`; + const verdaccioUrl = `http://localhost:${REGISTRY_PORT}`;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (52)
code/addons/a11y/package.json(1 hunks)code/addons/docs/package.json(1 hunks)code/addons/links/package.json(1 hunks)code/addons/onboarding/package.json(1 hunks)code/addons/pseudo-states/package.json(1 hunks)code/addons/themes/package.json(1 hunks)code/addons/vitest/package.json(1 hunks)code/builders/builder-vite/package.json(1 hunks)code/builders/builder-webpack5/package.json(1 hunks)code/core/package.json(1 hunks)code/core/src/bin/dispatcher.ts(1 hunks)code/core/src/common/js-package-manager/JsPackageManager.ts(2 hunks)code/core/src/common/js-package-manager/NPMProxy.ts(1 hunks)code/core/src/common/js-package-manager/PNPMProxy.ts(1 hunks)code/core/src/common/js-package-manager/Yarn1Proxy.ts(1 hunks)code/core/src/common/js-package-manager/Yarn2Proxy.ts(1 hunks)code/frameworks/angular/package.json(1 hunks)code/frameworks/ember/package.json(1 hunks)code/frameworks/html-vite/package.json(1 hunks)code/frameworks/nextjs-vite/package.json(1 hunks)code/frameworks/nextjs/package.json(1 hunks)code/frameworks/preact-vite/package.json(1 hunks)code/frameworks/react-native-web-vite/package.json(1 hunks)code/frameworks/react-vite/package.json(1 hunks)code/frameworks/react-webpack5/package.json(1 hunks)code/frameworks/server-webpack5/package.json(1 hunks)code/frameworks/svelte-vite/package.json(1 hunks)code/frameworks/sveltekit/package.json(1 hunks)code/frameworks/vue3-vite/package.json(1 hunks)code/frameworks/web-components-vite/package.json(1 hunks)code/lib/cli-sb/package.json(1 hunks)code/lib/cli-storybook/package.json(1 hunks)code/lib/codemod/package.json(1 hunks)code/lib/core-webpack/package.json(1 hunks)code/lib/create-storybook/package.json(1 hunks)code/lib/csf-plugin/package.json(1 hunks)code/lib/eslint-plugin/package.json(1 hunks)code/lib/react-dom-shim/package.json(1 hunks)code/presets/create-react-app/package.json(1 hunks)code/presets/react-webpack/package.json(1 hunks)code/presets/server-webpack/package.json(1 hunks)code/renderers/html/package.json(1 hunks)code/renderers/preact/package.json(1 hunks)code/renderers/react/package.json(1 hunks)code/renderers/server/package.json(1 hunks)code/renderers/svelte/package.json(1 hunks)code/renderers/vue3/package.json(1 hunks)code/renderers/web-components/package.json(1 hunks)scripts/package.json(1 hunks)scripts/run-registry.ts(8 hunks)scripts/utils/yarn.ts(1 hunks)scripts/verdaccio.yaml(1 hunks)
✅ Files skipped from review due to trivial changes (7)
- code/addons/onboarding/package.json
- code/frameworks/react-native-web-vite/package.json
- code/lib/codemod/package.json
- code/addons/pseudo-states/package.json
- code/lib/react-dom-shim/package.json
- code/frameworks/nextjs-vite/package.json
- code/renderers/web-components/package.json
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use ESLint and Prettier configurations that are enforced in the codebase
Files:
code/frameworks/vue3-vite/package.jsoncode/presets/react-webpack/package.jsoncode/renderers/html/package.jsoncode/lib/cli-sb/package.jsoncode/core/package.jsoncode/core/src/common/js-package-manager/NPMProxy.tscode/core/src/bin/dispatcher.tscode/frameworks/angular/package.jsoncode/frameworks/preact-vite/package.jsoncode/renderers/preact/package.jsonscripts/package.jsonscripts/utils/yarn.tscode/lib/cli-storybook/package.jsoncode/frameworks/html-vite/package.jsoncode/builders/builder-webpack5/package.jsoncode/presets/server-webpack/package.jsoncode/renderers/vue3/package.jsoncode/frameworks/nextjs/package.jsoncode/addons/links/package.jsoncode/core/src/common/js-package-manager/Yarn1Proxy.tscode/addons/vitest/package.jsoncode/builders/builder-vite/package.jsoncode/frameworks/web-components-vite/package.jsoncode/addons/docs/package.jsoncode/renderers/react/package.jsoncode/lib/eslint-plugin/package.jsoncode/addons/themes/package.jsoncode/lib/core-webpack/package.jsoncode/presets/create-react-app/package.jsoncode/addons/a11y/package.jsoncode/core/src/common/js-package-manager/PNPMProxy.tscode/core/src/common/js-package-manager/JsPackageManager.tscode/renderers/svelte/package.jsoncode/frameworks/svelte-vite/package.jsoncode/lib/create-storybook/package.jsoncode/core/src/common/js-package-manager/Yarn2Proxy.tscode/frameworks/sveltekit/package.jsonscripts/run-registry.tscode/lib/csf-plugin/package.jsoncode/frameworks/ember/package.jsoncode/renderers/server/package.jsoncode/frameworks/react-vite/package.jsoncode/frameworks/react-webpack5/package.jsoncode/frameworks/server-webpack5/package.json
code/**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier with --write flag to format code before committing
Run ESLint with yarn lint:js:cmd to check for linting issues and fix errors before committing
Files:
code/frameworks/vue3-vite/package.jsoncode/presets/react-webpack/package.jsoncode/renderers/html/package.jsoncode/lib/cli-sb/package.jsoncode/core/package.jsoncode/core/src/common/js-package-manager/NPMProxy.tscode/core/src/bin/dispatcher.tscode/frameworks/angular/package.jsoncode/frameworks/preact-vite/package.jsoncode/renderers/preact/package.jsoncode/lib/cli-storybook/package.jsoncode/frameworks/html-vite/package.jsoncode/builders/builder-webpack5/package.jsoncode/presets/server-webpack/package.jsoncode/renderers/vue3/package.jsoncode/frameworks/nextjs/package.jsoncode/addons/links/package.jsoncode/core/src/common/js-package-manager/Yarn1Proxy.tscode/addons/vitest/package.jsoncode/builders/builder-vite/package.jsoncode/frameworks/web-components-vite/package.jsoncode/addons/docs/package.jsoncode/renderers/react/package.jsoncode/lib/eslint-plugin/package.jsoncode/addons/themes/package.jsoncode/lib/core-webpack/package.jsoncode/presets/create-react-app/package.jsoncode/addons/a11y/package.jsoncode/core/src/common/js-package-manager/PNPMProxy.tscode/core/src/common/js-package-manager/JsPackageManager.tscode/renderers/svelte/package.jsoncode/frameworks/svelte-vite/package.jsoncode/lib/create-storybook/package.jsoncode/core/src/common/js-package-manager/Yarn2Proxy.tscode/frameworks/sveltekit/package.jsoncode/lib/csf-plugin/package.jsoncode/frameworks/ember/package.jsoncode/renderers/server/package.jsoncode/frameworks/react-vite/package.jsoncode/frameworks/react-webpack5/package.jsoncode/frameworks/server-webpack5/package.json
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode
Files:
code/core/src/common/js-package-manager/NPMProxy.tscode/core/src/bin/dispatcher.tsscripts/utils/yarn.tscode/core/src/common/js-package-manager/Yarn1Proxy.tscode/core/src/common/js-package-manager/PNPMProxy.tscode/core/src/common/js-package-manager/JsPackageManager.tscode/core/src/common/js-package-manager/Yarn2Proxy.tsscripts/run-registry.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{ts,tsx,js,jsx,mjs}: Use server-side logger from 'storybook/internal/node-logger' for Node.js code
Use client-side logger from 'storybook/internal/client-logger' for browser code
Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/core/src/common/js-package-manager/NPMProxy.tscode/core/src/bin/dispatcher.tscode/core/src/common/js-package-manager/Yarn1Proxy.tscode/core/src/common/js-package-manager/PNPMProxy.tscode/core/src/common/js-package-manager/JsPackageManager.tscode/core/src/common/js-package-manager/Yarn2Proxy.ts
code/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions that need to be tested from their modules
Files:
code/core/src/common/js-package-manager/NPMProxy.tscode/core/src/bin/dispatcher.tscode/core/src/common/js-package-manager/Yarn1Proxy.tscode/core/src/common/js-package-manager/PNPMProxy.tscode/core/src/common/js-package-manager/JsPackageManager.tscode/core/src/common/js-package-manager/Yarn2Proxy.ts
🧠 Learnings (7)
📓 Common learnings
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.
📚 Learning: 2025-10-02T09:22:13.215Z
Learnt from: JReinhold
Repo: storybookjs/storybook PR: 32607
File: code/package.json:243-243
Timestamp: 2025-10-02T09:22:13.215Z
Learning: The Storybook repository uses Yarn v^4 (any 4.x version) as the package manager, configured via .yarnrc.yml and package.json packageManager field. Specific patch versions within v4 can be upgraded as needed.
Applied to files:
code/frameworks/vue3-vite/package.jsoncode/presets/react-webpack/package.jsoncode/renderers/html/package.jsoncode/lib/cli-sb/package.jsoncode/core/package.jsoncode/core/src/bin/dispatcher.tscode/frameworks/angular/package.jsoncode/frameworks/preact-vite/package.jsoncode/renderers/preact/package.jsonscripts/package.jsonscripts/utils/yarn.tscode/lib/cli-storybook/package.jsoncode/frameworks/html-vite/package.jsoncode/builders/builder-webpack5/package.jsoncode/presets/server-webpack/package.jsoncode/renderers/vue3/package.jsoncode/frameworks/nextjs/package.jsoncode/addons/links/package.jsoncode/builders/builder-vite/package.jsoncode/frameworks/web-components-vite/package.jsoncode/addons/docs/package.jsoncode/renderers/react/package.jsoncode/addons/themes/package.jsoncode/lib/core-webpack/package.jsoncode/presets/create-react-app/package.jsoncode/addons/a11y/package.jsoncode/renderers/svelte/package.jsoncode/frameworks/svelte-vite/package.jsoncode/lib/create-storybook/package.jsoncode/frameworks/sveltekit/package.jsoncode/lib/csf-plugin/package.jsoncode/frameworks/ember/package.jsoncode/renderers/server/package.jsoncode/frameworks/react-vite/package.jsoncode/frameworks/react-webpack5/package.jsoncode/frameworks/server-webpack5/package.json
📚 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/presets/react-webpack/package.jsoncode/addons/a11y/package.jsoncode/frameworks/react-webpack5/package.json
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Use Yarn 4.9.1 as the package manager
Applied to files:
scripts/package.jsonscripts/utils/yarn.tscode/core/src/common/js-package-manager/Yarn1Proxy.ts
📚 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:
scripts/utils/yarn.ts
📚 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/frameworks/nextjs/package.jsoncode/renderers/react/package.json
📚 Learning: 2025-09-24T13:04:58.631Z
Learnt from: cylewaitforit
Repo: storybookjs/storybook PR: 31965
File: code/lib/eslint-plugin/src/rules/only-csf3.ts:31-33
Timestamp: 2025-09-24T13:04:58.631Z
Learning: The Storybook ESLint plugin supports ESLint v8.57+ where context.sourceCode is already available as a property since it was introduced in v8.40.0, so no fallback to context.getSourceCode() is needed in rules.
Applied to files:
code/lib/eslint-plugin/package.json
🧬 Code graph analysis (2)
code/core/src/bin/dispatcher.ts (2)
code/core/src/common/js-package-manager/JsPackageManager.ts (1)
packageJson(773-775)code/core/src/common/utils/command.ts (1)
executeCommand(41-53)
code/core/src/common/js-package-manager/JsPackageManager.ts (1)
code/core/src/manager-api/tests/versions.test.js (1)
latest(51-54)
🔇 Additional comments (46)
code/frameworks/nextjs/package.json (1)
19-19: Repository URL standardization looks good.The update to the
git+https://format aligns with the PR objective to standardize repository URLs across the codebase. This format is a standard npm convention for git repository references and poses no functional impact.code/builders/builder-webpack5/package.json (1)
20-20: Standardize repository URL to git+https convention.This update aligns the repository URL with the npm/yarn standard convention for Git HTTPS URLs. The change is safe and part of the broader standardization effort mentioned in the PR objectives.
code/frameworks/svelte-vite/package.json (1)
19-19: Repository URL standardization looks good.The change from
https://togit+https://aligns with the repository URL standardization applied across the codebase and follows the documented Git protocol format for package.json.code/frameworks/sveltekit/package.json (1)
21-21: Repository URL format standardization looks good.The update from
https://togit+https://protocol prefix is a valid npm registry format that explicitly indicates the repository is a git repository. This aligns with the PR's broader effort to standardize repository URLs across package.json files.code/frameworks/preact-vite/package.json (1)
19-19: Valid repository URL format standardization.The repository URL has been updated from
https://togit+https://format, which is a standard npm registry convention for explicitly specifying git protocol-based repositories. This format is fully supported by Yarn v^4 (the package manager used by this repository) and aligns with the broader standardization effort across the codebase.code/frameworks/react-webpack5/package.json (1)
19-19: Repository URL standardization looks good.The
git+https://prefix update aligns with the PR's effort to standardize repository URLs across package.json files. This is a valid npm registry format for Git URLs.code/frameworks/web-components-vite/package.json (1)
21-21: Repository URL standardization looks good.The
git+https://prefix is a standard and explicit format for Git repository URLs in package.json. This change aligns with the PR's intent to standardize repository URLs across the codebase and does not introduce any functional issues.code/lib/csf-plugin/package.json (1)
14-14: Repository URL standardization aligns with PR objectives.The update to use
git+https://is consistent with the repository-wide URL standardization mentioned in the PR context and properly formatted for npm's git protocol resolution.code/lib/create-storybook/package.json (1)
17-17: Repository URL standardization is correct.The change from
https://togit+https://explicitly declares the git protocol for the repository field. This is a valid npm package.json format and aligns with the broader repository-wide URL standardization effort mentioned in the PR summary.code/lib/eslint-plugin/package.json (1)
15-19: Repository URL standardized to git+ format.The change from
https://github.com/storybookjs/storybooktogit+https://github.com/storybookjs/storybook.gitaligns with npm conventions for git-based repositories and is consistent with the broader standardization mentioned in the PR summary.To confirm this change is part of a cohesive standardization across the repository, you may want to verify that similar updates have been applied to other
package.jsonfiles. If you'd like, I can run a quick check to see the extent of this standardization across the codebase.code/frameworks/html-vite/package.json (1)
19-19: Repository URL format standardization looks good.The change from
https://togit+https://is a valid standardization pattern. Thegit+scheme prefix is widely supported by npm/yarn and makes the protocol intent explicit without affecting repository resolution.This aligns with the PR's broader effort to standardize repository URLs across package.json files in the monorepo.
code/frameworks/vue3-vite/package.json (1)
19-19: Repository URL standardization looks good.The update from
https://togit+https://follows npm conventions for explicit git URL identification and aligns with the PR's broader standardization effort across multiple package.json files.code/frameworks/react-vite/package.json (1)
17-21: Standardize repository URL to git+https protocol — LGTM.The repository URL has been updated to use the
git+https://protocol prefix, which is a standard npm/yarn convention for explicitly declaring git-based repositories. This aligns with the project-wide standardization mentioned in the PR.code/addons/docs/package.json (2)
31-31: Repository URL format standardization looks good.The change from
https://togit+https://is a valid Git protocol specification that improves compatibility with Git tools and package managers.
117-117: Suspicious gitHead value.The
gitHeadfield ends withlinstead of a valid hexadecimal character, suggesting potential data corruption:a8e7fd8a655c69780bc20b9749d2699e45beae1l.Verify that this is not a typo or data corruption issue. If this value is auto-generated, ensure the generation process is working correctly.
code/frameworks/server-webpack5/package.json (2)
17-17: Repository URL standardization approved.The
git+https://format is consistent with the broader PR pattern and improves Git/npm registry compatibility.
61-61: Systematic gitHead corruption detected.This file also has the same suspicious
gitHeadending:a8e7fd8a655c69780bc20b9749d2699e45beae1l. This suggests a systematic issue affecting multiple package.json files in the PR.Verify the source of the
gitHeadvalues. If this is auto-generated during the build/publish process, investigate the generation logic.code/renderers/svelte/package.json (1)
17-17: Repository URL update approved.code/addons/themes/package.json (1)
27-27: Repository URL update approved.code/addons/a11y/package.json (1)
28-28: Repository URL update approved.code/addons/links/package.json (1)
20-20: Repository URL update approved.code/lib/cli-storybook/package.json (1)
19-19: Repository URL update approved.code/renderers/html/package.json (1)
18-18: Repository URL update approved.code/renderers/server/package.json (1)
15-19: Repository URL normalization looks good
repository.urluses thegit+httpsform and matches the main Storybook repo; this is consistent with other packages and has no runtime effect.code/frameworks/angular/package.json (1)
16-20: Consistent repository URL formatUsing
git+https://github.com/storybookjs/storybook.gitkeeps this package aligned with the rest of the monorepo’s metadata; no functional impact.code/lib/cli-sb/package.json (1)
17-21: CLI package metadata aligned with repo standard
repository.urlnow uses thegit+httpsscheme, consistent with the rest of the packages and npm conventions.code/addons/vitest/package.json (1)
23-27: Addon repository URL updated correctlyThe Vitest addon’s
repository.urlis now in the samegit+httpsformat as other Storybook packages; looks correct.code/core/package.json (1)
26-30: Core package repository URL standardizedSwitching
repository.urltogit+httpsmatches the rest of the workspace manifests and is appropriate for npm metadata.code/builders/builder-vite/package.json (1)
18-22: Builder metadata uses the new git+ URL
@storybook/builder-vitenow pointsrepository.urlat thegit+httpsStorybook repo, in line with the rest of the PR’s metadata updates.code/presets/create-react-app/package.json (1)
12-16: Preset repository URL change is safeThe CRA preset’s
repository.urlnow usesgit+https, consistent with other Storybook packages; no runtime effect.code/renderers/preact/package.json (1)
18-18: LGTM! Repository URL standardization.The addition of the
git+prefix aligns with npm's canonical format for Git repository URLs.code/renderers/vue3/package.json (1)
18-18: LGTM! Repository URL standardization.The addition of the
git+prefix aligns with npm's canonical format for Git repository URLs.code/renderers/react/package.json (1)
14-14: LGTM! Repository URL standardization.The addition of the
git+prefix aligns with npm's canonical format for Git repository URLs.code/presets/server-webpack/package.json (1)
14-14: LGTM! Repository URL standardization.The addition of the
git+prefix aligns with npm's canonical format for Git repository URLs.code/frameworks/ember/package.json (1)
18-18: LGTM! Repository URL standardization.The addition of the
git+prefix aligns with npm's canonical format for Git repository URLs.code/presets/react-webpack/package.json (1)
14-14: LGTM! Repository URL standardization.The addition of the
git+prefix aligns with npm's canonical format for Git repository URLs.code/lib/core-webpack/package.json (1)
14-14: LGTM! Repository URL standardization.The addition of the
git+prefix aligns with npm's canonical format for Git repository URLs.scripts/run-registry.ts (2)
71-85: Proxy routing logic is well-structured.The proxy correctly routes requests based on package name matching, with special handling for PUT requests (publish operations). Using
decodeURIComponenthandles scoped packages that may be URL-encoded.
181-207: Workspace dependency resolution looks correct.The function properly handles all
workspace:specifier variants (^,~, and exact) and correctly substitutes them with the published version.scripts/verdaccio.yaml (2)
5-6: No-auth configuration appropriate for local testing.The empty
auth: {}configuration removes authentication requirements. The comment clearly documents this is for e2e testing only. This is acceptable for a local development registry.
24-26: Offline publishing enables disconnected workflow.The
allow_offline: truesetting allows publishing without an npm connection, which aligns with the PR's goal of removing external Nx Cloud dependencies.scripts/package.json (1)
178-178: Verdaccio promoted to required dependency.Moving
verdacciofromoptionalDependenciestodependenciesaligns with the new Verdaccio-backed publish workflow. The removal of@verdaccio/typesandverdaccio-auth-memoryfrom optional dependencies is consistent with the simplified auth configuration inverdaccio.yaml.Version
^6.2.3is stable and current (latest is 6.2.4, a minor patch ahead). The caret constraint appropriately allows compatible updates.code/core/src/common/js-package-manager/PNPMProxy.ts (1)
218-223: Aligns pnpm info with package manager working directoryUsing
cwd: this.cwdhere is consistent with other PNPM commands and the sharedJsPackageManagercontract, sopnpm infowill respect the same repo root/config as installs and internal commands.code/core/src/common/js-package-manager/NPMProxy.ts (1)
216-220: Consistent cwd usage for npm info lookupsAdding
cwd: this.cwdmakesnpm inforesolution consistent with installs and internal commands, especially whenJsPackageManageris initialized with a non-defaultcwd.code/core/src/common/js-package-manager/Yarn2Proxy.ts (1)
265-269: Yarn 2 version lookup now respects manager cwdSetting
cwd: this.cwdforyarn npm infoaligns this path with other Yarn2Proxy operations and the other package-manager proxies, avoiding surprises when the manager is constructed with a custom working directory.code/core/src/common/js-package-manager/Yarn1Proxy.ts (1)
172-176: Yarn 1 info now runs from the intended working directoryAdding
cwd: this.cwdto theyarn infoinvocation keeps version queries aligned with how installs and other internal commands are scoped. This matches the pattern used across all package-manager proxies.
| if (latestInRange?.includes('-local.')) { | ||
| return `${packageName}@local`; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the file and get context around the mentioned lines
fd -t f "JsPackageManager.ts" code/Repository: storybookjs/storybook
Length of output: 124
🏁 Script executed:
# Read the file to understand the context
wc -l code/core/src/common/js-package-manager/JsPackageManager.tsRepository: storybookjs/storybook
Length of output: 128
🏁 Script executed:
# Get the specific sections mentioned in the review
sed -n '375,395p' code/core/src/common/js-package-manager/JsPackageManager.tsRepository: storybookjs/storybook
Length of output: 653
🏁 Script executed:
# Get lines 448-449
sed -n '440,455p' code/core/src/common/js-package-manager/JsPackageManager.tsRepository: storybookjs/storybook
Length of output: 570
🏁 Script executed:
# Find the getVersionedPackages function
ast-grep --pattern 'getVersionedPackages($$$) {
$$$
}'Repository: storybookjs/storybook
Length of output: 47
🏁 Script executed:
# Find the getVersion function
ast-grep --pattern 'getVersion($$$) {
$$$
}'Repository: storybookjs/storybook
Length of output: 47
🏁 Script executed:
# Find all calls to getVersion and getVersionedPackages
rg "getVersion\(|getVersionedPackages\(" code/ --type ts --type tsxRepository: storybookjs/storybook
Length of output: 92
🏁 Script executed:
# Fix rg search - use correct flags
rg "getVersion\(|getVersionedPackages\(" code --type tsRepository: storybookjs/storybook
Length of output: 2067
🏁 Script executed:
# Let me get the full function context for getVersionedPackages starting around line 375
sed -n '350,400p' code/core/src/common/js-package-manager/JsPackageManager.tsRepository: storybookjs/storybook
Length of output: 1822
🏁 Script executed:
# Get the full getVersion function context around 448
sed -n '425,460p' code/core/src/common/js-package-manager/JsPackageManager.tsRepository: storybookjs/storybook
Length of output: 1289
🏁 Script executed:
# Find function definitions
grep -n "getVersion\|getVersionedPackages" code/core/src/common/js-package-manager/JsPackageManager.ts | head -20Repository: storybookjs/storybook
Length of output: 422
🏁 Script executed:
# Look at the actual callers more closely
sed -n '1,20p' code/lib/create-storybook/src/generators/REACT_NATIVE/index.tsRepository: storybookjs/storybook
Length of output: 905
🏁 Script executed:
# Find the context where getVersionedPackages is called in baseGenerator
grep -n "getVersionedPackages" code/lib/create-storybook/src/generators/baseGenerator.tsRepository: storybookjs/storybook
Length of output: 139
🏁 Script executed:
# See how the result is used after getVersionedPackages call
sed -n '70,120p' code/lib/create-storybook/src/generators/baseGenerator.tsRepository: storybookjs/storybook
Length of output: 1687
🏁 Script executed:
# Look at test file to see how 'local' is tested
cat -n code/core/src/common/js-package-manager/JsPackageManager.test.ts | grep -A10 -B5 "local"Repository: storybookjs/storybook
Length of output: 47
🏁 Script executed:
# Check how the results from getVersionedPackages are used - see what happens with the version strings
rg "getVersionedPackages" code/lib/create-storybook/src/generators/baseGenerator.ts -A 10Repository: storybookjs/storybook
Length of output: 478
🏁 Script executed:
# Check the CLI helpers to see how getVersion is used
sed -n '1,100p' code/core/src/cli/helpers.ts | grep -A5 -B5 "getVersion"Repository: storybookjs/storybook
Length of output: 817
🏁 Script executed:
# Check test file more thoroughly for null scenarios
cat code/core/src/common/js-package-manager/JsPackageManager.test.ts | grep -n "null\|undefined\|local" -iRepository: storybookjs/storybook
Length of output: 47
🏁 Script executed:
# Look at latestVersion implementation to understand when it returns null
sed -n '475,550p' code/core/src/common/js-package-manager/JsPackageManager.tsRepository: storybookjs/storybook
Length of output: 2854
🏁 Script executed:
# Check if there are any guards in callers of getVersionedPackages
grep -B5 -A15 "getVersionedPackages" code/lib/create-storybook/src/generators/baseGenerator.ts | head -40Repository: storybookjs/storybook
Length of output: 759
🏁 Script executed:
# See the full context of line 388 in getVersionedPackages
sed -n '375,395p' code/core/src/common/js-package-manager/JsPackageManager.tsRepository: storybookjs/storybook
Length of output: 653
🏁 Script executed:
# Check how dependencyCollector.addDevDependencies handles the version specs
fd -t f "dependencyCollector\|DependencyCollector" code/ --type ts | head -5Repository: storybookjs/storybook
Length of output: 234
🏁 Script executed:
# Let me verify what happens when @^null would be passed to npm/yarn
# Check if there are any tests that explicitly test error scenarios
grep -n "null\|error\|Error" code/core/src/common/js-package-manager/JsPackageManager.test.ts | head -20Repository: storybookjs/storybook
Length of output: 200
🏁 Script executed:
# See if there are mock/stub tests that return null from latestVersion
grep -n "latestVersion\|Mock\|mock" code/core/src/common/js-package-manager/JsPackageManager.test.ts | head -30Repository: storybookjs/storybook
Length of output: 571
🏁 Script executed:
# Check npm/yarn spec parsing to confirm @local is valid
rg "@local|dist-tag" code --type ts -i | head -10Repository: storybookjs/storybook
Length of output: 165
🏁 Script executed:
# Final verification: check if there are any error handling tests or scenarios
sed -n '1,100p' code/core/src/common/js-package-manager/JsPackageManager.test.tsRepository: storybookjs/storybook
Length of output: 1777
Add null-guard for latestInRange in getVersionedPackages to prevent invalid package specs
In getVersionedPackages, if latestVersion() returns null (due to network errors or registry issues) and the version doesn't contain -local., line 388 will produce an invalid spec like @^null. Although getVersion() guards against this with an explicit null check, getVersionedPackages() lacks protection.
Add a simple null check after the -local. guard:
if (latestInRange?.includes('-local.')) {
return `${packageName}@local`;
}
+if (!latestInRange) {
+ return currentVersion ? `${packageName}@${currentVersion}` : packageName;
+}
+
if (isLatestStableRelease || !currentVersion) {
return `${packageName}@^${latestInRange}`;
}📝 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.
| if (latestInRange?.includes('-local.')) { | |
| return `${packageName}@local`; | |
| } | |
| if (latestInRange?.includes('-local.')) { | |
| return `${packageName}@local`; | |
| } | |
| if (!latestInRange) { | |
| return currentVersion ? `${packageName}@${currentVersion}` : packageName; | |
| } | |
| if (isLatestStableRelease || !currentVersion) { | |
| return `${packageName}@^${latestInRange}`; | |
| } |
🤖 Prompt for AI Agents
In code/core/src/common/js-package-manager/JsPackageManager.ts around lines
383-385, the code assumes latestInRange is non-null which can produce invalid
specs like "@^null"; add a null-guard immediately after the existing '-local.'
check so that if latestInRange is null you skip versioning for that package
(e.g., continue the loop or push the bare packageName) instead of constructing a
spec using "null".
Closes #
What I did
Remove the nx cloud connection in circel CI, as two seperate connection seem to trip up NX.
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
Chores
Chores (dev tooling)
✏️ Tip: You can customize this high-level summary in your review settings.