Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions scripts/tasks/check.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ const amountOfVCPUs = 8;

const parallel = `--parallel=${process.env.CI ? amountOfVCPUs - 1 : maxConcurrentTasks}`;

const linkCommand = `yarn nx run-many -t check ${parallel}`;
const nolinkCommand = `yarn nx run-many -t check -c production ${parallel}`;
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`;
Comment on lines +11 to +12
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Read the check.ts file
wc -l scripts/tasks/check.ts
echo "---"
cat -n scripts/tasks/check.ts

Repository: 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/null

Repository: 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 -20

Repository: 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 -10

Repository: 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.


export const check: Task = {
description: 'Typecheck the source code of the monorepo',
Expand Down
4 changes: 2 additions & 2 deletions scripts/tasks/compile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@ export const compile: Task = {
return false;
}
},
async run({ codeDir }, { link, dryRun, debug, prod, skipCache }) {
async run({ codeDir }, { link, dryRun, debug, prod }) {
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 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.ts

Repository: 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.ts

Repository: 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 -50

Repository: 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.ts

Repository: 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.

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`,
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 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 -30

Repository: storybookjs/storybook

Length of output: 927


🏁 Script executed:

#!/bin/bash
# List files in scripts/tasks directory
ls -la scripts/tasks/ | head -20

Repository: storybookjs/storybook

Length of output: 1260


🏁 Script executed:

#!/bin/bash
cat -n scripts/tasks/compile.ts

Repository: storybookjs/storybook

Length of output: 2349


🏁 Script executed:

#!/bin/bash
cat -n scripts/tasks/check.ts

Repository: 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.

Comment on lines +40 to +44
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

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

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:

  1. Removing the skipCache option from the task options if it's no longer needed
  2. Or documenting that this option is deprecated/unused for the compile task

Copilot uses AI. Check for mistakes.
{ cwd: ROOT_DIRECTORY },
{
startMessage: '🥾 Bootstrapping',
Expand Down
Loading