Skip to content

CLI: Drop contextual dependencies in create-storybook #32134

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ndelangen merged 8 commits into from
Sep 18, 2025
+79 −170
Merged

CLI: Drop contextual dependencies in create-storybook #32134

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5d00cfd
fix(create-storybook): drop contextual dependencies
43081j Jul 27, 2025
a7bb7d0
Merge branch 'next' into config-files-context
ndelangen Sep 15, 2025
3408388
test: Update vitestConfigFiles.test.ts to mock fs/promises with dynam…
ndelangen Sep 15, 2025
19a909d
Merge branch 'next' into config-files-context
43081j Sep 16, 2025
6a3ae4b
chore: run format
43081j Sep 16, 2025
6756ac0
test: mock find.any
43081j Sep 16, 2025
b2b20b4
Merge branch 'next' into pr/43081j/32134
ndelangen Sep 17, 2025
12ded7f
Refactor `coerce` function for improved readability in `vitestConfigF…
ndelangen Sep 17, 2025
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
156 changes: 37 additions & 119 deletions code/lib/create-storybook/src/ink/steps/checks/vitestConfigFiles.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
import * as fs from 'node:fs/promises';

import { describe, expect, it } from 'vitest';

import * as babel from 'storybook/internal/babel';
import { describe, expect, it, vi } from 'vitest';

import * as find from 'empathic/find';

import { vitestConfigFiles } from './vitestConfigFiles';

const liveContext: any = { babel, empathic: find, fs };
vi.mock('empathic/find', () => ({
any: vi.fn(),
}));

const fileMocks = {
'vitest.config.ts': `
Expand Down Expand Up @@ -70,101 +68,63 @@ const fileMocks = {
`,
};

const mockContext: any = {
...liveContext,
empathic: { any: ([name]: string[]) => name },
fs: {
readFile: async (path: keyof typeof fileMocks) => fileMocks[path],
},
};
vi.mock(import('node:fs/promises'), async (importOriginal) => {
const actual = await importOriginal();
return {
...actual,
readFile: vi
.fn()
.mockImplementation((filePath) => fileMocks[filePath as keyof typeof fileMocks]),
};
});
Comment on lines +71 to +79

@coderabbitai coderabbitai Bot Sep 17, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Invalid vi.mock target and non-async readFile mock

First arg must be the module id string. Also return a Promise for async APIs and use spy: true per rules.

-vi.mock(import('node:fs/promises'), async (importOriginal) => {
-  const actual = await importOriginal();
-  return {
-    ...actual,
-    readFile: vi
-      .fn()
-      .mockImplementation((filePath) => fileMocks[filePath as keyof typeof fileMocks]),
-  };
-});
+vi.mock(
+  'node:fs/promises',
+  async () => {
+    const actual = await vi.importActual<typeof import('node:fs/promises')>('node:fs/promises');
+    return {
+      ...actual,
+      readFile: vi.fn(async (filePath: string) =>
+        Promise.resolve(fileMocks[filePath as keyof typeof fileMocks])
+      ),
+    };
+  },
+  { spy: true }
+);
📝 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.

Suggested change
vi.mock(import('node:fs/promises'), async (importOriginal) => {
const actual = await importOriginal();
return {
...actual,
readFile: vi
.fn()
.mockImplementation((filePath) => fileMocks[filePath as keyof typeof fileMocks]),
};
});
vi.mock(
'node:fs/promises',
async () => {
const actual = await vi.importActual<typeof import('node:fs/promises')>('node:fs/promises');
return {
...actual,
readFile: vi.fn(async (filePath: string) =>
Promise.resolve(fileMocks[filePath as keyof typeof fileMocks])
),
};
},
{ spy: true }
);
🤖 Prompt for AI Agents 
In code/lib/create-storybook/src/ink/steps/checks/vitestConfigFiles.test.ts
around lines 71-79, the vi.mock call is using import(...) as the first arg and
returning a non-async readFile mock; change it to mock the module by its string
id ('node:fs/promises'), use vi.importActual to get the real implementation,
return an object shaped like the module (exporting the actual members) and
ensure readFile is mocked as an async function (use
vi.fn().mockResolvedValue(...) or mockImplementation returning a Promise) and
pass { spy: true } in the vi.mock options so spies are allowed; ensure the mock
factory returns the module object (or Promise of it) as required by Vitest.


const mockContext: any = {};

const coerce =
(from: string, to: string) =>
async ([name]: string[]) =>
([name]: string[]) =>
name.includes(from) ? to : name;

const state: any = {
directory: '.',
};

// TODO @ghengeveld, I am in the process of removing the context
describe.skip('these tests need to be updated', () => {
it('should run properly with live dependencies', async () => {
const result = await vitestConfigFiles.condition(liveContext, state);
expect(result).toEqual({ type: 'compatible' });
});

describe('vitestConfigFiles', () => {
it('should run properly with mock dependencies', async () => {
const result = await vitestConfigFiles.condition(mockContext, state);
expect(result).toEqual({ type: 'compatible' });
});

it('should disallow missing dependencies', async () => {
const result = await vitestConfigFiles.condition({} as any, state);
expect(result).toEqual({
type: 'incompatible',
reasons: ['Missing babel on context', 'Missing empathic on context', 'Missing fs on context'],
});
});

describe('Check Vitest workspace files', () => {
it('should disallow JSON workspace file', async () => {
const result = await vitestConfigFiles.condition(
{
...mockContext,
empathic: {
up: coerce('workspace', 'vitest.workspace.json'),
},
},
state
);
vi.mocked(find.any).mockImplementation(coerce('workspace', 'vitest.workspace.json'));
const result = await vitestConfigFiles.condition(mockContext, state);
expect(result).toEqual({
type: 'incompatible',
reasons: ['Cannot auto-update JSON workspace file: vitest.workspace.json'],
});
});

it('should disallow invalid workspace file', async () => {
const result = await vitestConfigFiles.condition(
{
...mockContext,
empathic: {
up: coerce('workspace', 'invalidWorkspace.ts'),
},
},
state
);
vi.mocked(find.any).mockImplementation(coerce('workspace', 'invalidWorkspace.ts'));
const result = await vitestConfigFiles.condition(mockContext, state);
expect(result).toEqual({
type: 'incompatible',
reasons: ['Found an invalid workspace config file: invalidWorkspace.ts'],
});
});

it('should allow defineWorkspace syntax', async () => {
const result = await vitestConfigFiles.condition(
{
...mockContext,
empathic: {
up: coerce('workspace', 'defineWorkspace.ts'),
},
},
state
);
vi.mocked(find.any).mockImplementation(coerce('workspace', 'defineWorkspace.ts'));
const result = await vitestConfigFiles.condition(mockContext, state);
expect(result).toEqual({
type: 'compatible',
});
});

it('should disallow invalid defineWorkspace syntax', async () => {
const result = await vitestConfigFiles.condition(
{
...mockContext,
empathic: {
up: coerce('workspace', 'defineWorkspace-invalid.ts'),
},
},
state
);
vi.mocked(find.any).mockImplementation(coerce('workspace', 'defineWorkspace-invalid.ts'));
const result = await vitestConfigFiles.condition(mockContext, state);
expect(result).toEqual({
type: 'incompatible',
reasons: ['Found an invalid workspace config file: defineWorkspace-invalid.ts'],
Expand All @@ -174,93 +134,51 @@ describe.skip('these tests need to be updated', () => {

describe('Check Vitest config files', () => {
it('should disallow CommonJS config file', async () => {
const result = await vitestConfigFiles.condition(
{
...mockContext,
empathic: {
up: coerce('config', 'vitest.config.cjs'),
},
},
state
);
vi.mocked(find.any).mockImplementation(coerce('config', 'vitest.config.cjs'));
const result = await vitestConfigFiles.condition(mockContext, state);
expect(result).toEqual({
type: 'incompatible',
reasons: ['Cannot auto-update CommonJS config file: vitest.config.cjs'],
});
});

it('should disallow invalid config file', async () => {
const result = await vitestConfigFiles.condition(
{
...mockContext,
empathic: {
up: coerce('config', 'invalidConfig.ts'),
},
},
state
);
vi.mocked(find.any).mockImplementation(coerce('config', 'invalidConfig.ts'));
const result = await vitestConfigFiles.condition(mockContext, state);
expect(result).toEqual({
type: 'incompatible',
reasons: ['Found an invalid Vitest config file: invalidConfig.ts'],
});
});

it('should allow existing test config option', async () => {
const result = await vitestConfigFiles.condition(
{
...mockContext,
empathic: {
up: coerce('config', 'testConfig.ts'),
},
},
state
);
vi.mocked(find.any).mockImplementation(coerce('config', 'testConfig.ts'));
const result = await vitestConfigFiles.condition(mockContext, state);
expect(result).toEqual({
type: 'compatible',
});
});

it('should disallow invalid test config option', async () => {
const result = await vitestConfigFiles.condition(
{
...mockContext,
empathic: {
up: coerce('config', 'testConfig-invalid.ts'),
},
},
state
);
vi.mocked(find.any).mockImplementation(coerce('config', 'testConfig-invalid.ts'));
const result = await vitestConfigFiles.condition(mockContext, state);
expect(result).toEqual({
type: 'incompatible',
reasons: ['Found an invalid Vitest config file: testConfig-invalid.ts'],
});
});

it('should allow existing test.workspace config option', async () => {
const result = await vitestConfigFiles.condition(
{
...mockContext,
empathic: {
up: coerce('config', 'workspaceConfig.ts'),
},
},
state
);
vi.mocked(find.any).mockImplementation(coerce('config', 'workspaceConfig.ts'));
const result = await vitestConfigFiles.condition(mockContext, state);
expect(result).toEqual({
type: 'compatible',
});
});

it('should disallow invalid test.workspace config option', async () => {
const result = await vitestConfigFiles.condition(
{
...mockContext,
empathic: {
up: coerce('config', 'workspaceConfig-invalid.ts'),
},
},
state
);
vi.mocked(find.any).mockImplementation(coerce('config', 'workspaceConfig-invalid.ts'));
const result = await vitestConfigFiles.condition(mockContext, state);
expect(result).toEqual({
type: 'incompatible',
reasons: ['Found an invalid Vitest config file: workspaceConfig-invalid.ts'],
Expand Down
93 changes: 42 additions & 51 deletions code/lib/create-storybook/src/ink/steps/checks/vitestConfigFiles.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,60 +88,51 @@ export const isValidWorkspaceConfigFile: (fileContents: string, babel: any) => b
* - No -> exit
*/
export const vitestConfigFiles: Check = {
condition: async (context, state) => {
const deps = ['babel', 'empathic', 'fs'];
if (babel && find && fs) {
const reasons = [];

const projectRoot = getProjectRoot();

const vitestWorkspaceFile = find.any(
['ts', 'js', 'json'].flatMap((ex) => [`vitest.workspace.${ex}`, `vitest.projects.${ex}`]),
{ cwd: state.directory, last: projectRoot }
);
if (vitestWorkspaceFile?.endsWith('.json')) {
reasons.push(`Cannot auto-update JSON workspace file: ${vitestWorkspaceFile}`);
} else if (vitestWorkspaceFile) {
const fileContents = await fs.readFile(vitestWorkspaceFile, 'utf8');
if (!isValidWorkspaceConfigFile(fileContents, babel)) {
reasons.push(`Found an invalid workspace config file: ${vitestWorkspaceFile}`);
}
condition: async (_context, state) => {
const reasons = [];

const projectRoot = getProjectRoot();

const vitestWorkspaceFile = find.any(
['ts', 'js', 'json'].flatMap((ex) => [`vitest.workspace.${ex}`, `vitest.projects.${ex}`]),
{ cwd: state.directory, last: projectRoot }
);
if (vitestWorkspaceFile?.endsWith('.json')) {
reasons.push(`Cannot auto-update JSON workspace file: ${vitestWorkspaceFile}`);
} else if (vitestWorkspaceFile) {
const fileContents = await fs.readFile(vitestWorkspaceFile, 'utf8');
if (!isValidWorkspaceConfigFile(fileContents, babel)) {
reasons.push(`Found an invalid workspace config file: ${vitestWorkspaceFile}`);
}
}
Comment on lines +103 to +107

@coderabbitai coderabbitai Bot Sep 17, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Harden workspace parsing with try/catch; drop unused babel arg

Avoids unhandled exceptions on unreadable/invalid files and removes the unused parameter.

-      const fileContents = await fs.readFile(vitestWorkspaceFile, 'utf8');
-      if (!isValidWorkspaceConfigFile(fileContents, babel)) {
-        reasons.push(`Found an invalid workspace config file: ${vitestWorkspaceFile}`);
-      }
+      try {
+        const fileContents = await fs.readFile(vitestWorkspaceFile, 'utf8');
+        if (!isValidWorkspaceConfigFile(fileContents)) {
+          reasons.push(`Found an invalid workspace config file: ${vitestWorkspaceFile}`);
+        }
+      } catch {
+        reasons.push(`Error reading/parsing workspace config file: ${vitestWorkspaceFile}`);
+      }

Outside this hunk, update the helper’s signature to match usage:

-export const isValidWorkspaceConfigFile: (fileContents: string, babel: any) => boolean = (
-  fileContents
-) => {
+export const isValidWorkspaceConfigFile = (fileContents: string): boolean => {
📝 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.

Suggested change
const fileContents = await fs.readFile(vitestWorkspaceFile, 'utf8');
if (!isValidWorkspaceConfigFile(fileContents, babel)) {
reasons.push(`Found an invalid workspace config file: ${vitestWorkspaceFile}`);
}
}
try {
const fileContents = await fs.readFile(vitestWorkspaceFile, 'utf8');
if (!isValidWorkspaceConfigFile(fileContents)) {
reasons.push(`Found an invalid workspace config file: ${vitestWorkspaceFile}`);
}
} catch {
reasons.push(`Error reading/parsing workspace config file: ${vitestWorkspaceFile}`);
}
}
Suggested change
const fileContents = await fs.readFile(vitestWorkspaceFile, 'utf8');
if (!isValidWorkspaceConfigFile(fileContents, babel)) {
reasons.push(`Found an invalid workspace config file: ${vitestWorkspaceFile}`);
}
}
export const isValidWorkspaceConfigFile = (fileContents: string): boolean => {
🤖 Prompt for AI Agents 
In code/lib/create-storybook/src/ink/steps/checks/vitestConfigFiles.tsx around
lines 103 to 107, wrap the fs.readFile + isValidWorkspaceConfigFile call in a
try/catch so unreadable or invalid files don't throw (on error push a
descriptive reason like `Unable to read/parse workspace config:
${vitestWorkspaceFile}`); remove the unused `babel` argument from the
isValidWorkspaceConfigFile call here and update the helper function's signature
and all its call sites elsewhere to drop that parameter so signatures and usages
remain consistent.


const vitestConfigFile = find.any(
['ts', 'js', 'tsx', 'jsx', 'cts', 'cjs', 'mts', 'mjs'].map((ex) => `vitest.config.${ex}`),
{ cwd: state.directory, last: projectRoot }
);
if (vitestConfigFile?.endsWith('.cts') || vitestConfigFile?.endsWith('.cjs')) {
reasons.push(`Cannot auto-update CommonJS config file: ${vitestConfigFile}`);
} else if (vitestConfigFile) {
let isValidVitestConfig = false;
const configContent = await fs.readFile(vitestConfigFile, 'utf8');
const parsedConfig = babel.babelParse(configContent);
babel.traverse(parsedConfig, {
ExportDefaultDeclaration(path) {
if (
isDefineConfigExpression(path.node.declaration) &&
isSafeToExtendWorkspace(path.node.declaration as CallExpression)
) {
isValidVitestConfig = true;
}
},
});
if (!isValidVitestConfig) {
reasons.push(`Found an invalid Vitest config file: ${vitestConfigFile}`);
}
const vitestConfigFile = find.any(
['ts', 'js', 'tsx', 'jsx', 'cts', 'cjs', 'mts', 'mjs'].map((ex) => `vitest.config.${ex}`),
{ cwd: state.directory, last: projectRoot }
);
if (vitestConfigFile?.endsWith('.cts') || vitestConfigFile?.endsWith('.cjs')) {
reasons.push(`Cannot auto-update CommonJS config file: ${vitestConfigFile}`);
} else if (vitestConfigFile) {
let isValidVitestConfig = false;
const configContent = await fs.readFile(vitestConfigFile, 'utf8');
const parsedConfig = babel.babelParse(configContent);
babel.traverse(parsedConfig, {
ExportDefaultDeclaration(path) {
if (
isDefineConfigExpression(path.node.declaration) &&
isSafeToExtendWorkspace(path.node.declaration as CallExpression)
) {
isValidVitestConfig = true;
}
},
});
if (!isValidVitestConfig) {
reasons.push(`Found an invalid Vitest config file: ${vitestConfigFile}`);
}

return reasons.length
? { type: CompatibilityType.INCOMPATIBLE, reasons }
: { type: CompatibilityType.COMPATIBLE };
}
Comment on lines +109 to 132

@coderabbitai coderabbitai Bot Sep 17, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Guard Vitest config read/parse with try/catch

Prevents crashes on IO/parse errors and surfaces actionable reasons.

-    } else if (vitestConfigFile) {
-      let isValidVitestConfig = false;
-      const configContent = await fs.readFile(vitestConfigFile, 'utf8');
-      const parsedConfig = babel.babelParse(configContent);
-      babel.traverse(parsedConfig, {
-        ExportDefaultDeclaration(path) {
-          if (
-            isDefineConfigExpression(path.node.declaration) &&
-            isSafeToExtendWorkspace(path.node.declaration as CallExpression)
-          ) {
-            isValidVitestConfig = true;
-          }
-        },
-      });
-      if (!isValidVitestConfig) {
-        reasons.push(`Found an invalid Vitest config file: ${vitestConfigFile}`);
-      }
-    }
+    } else if (vitestConfigFile) {
+      try {
+        let isValidVitestConfig = false;
+        const configContent = await fs.readFile(vitestConfigFile, 'utf8');
+        const parsedConfig = babel.babelParse(configContent);
+        babel.traverse(parsedConfig, {
+          ExportDefaultDeclaration(path) {
+            if (
+              isDefineConfigExpression(path.node.declaration) &&
+              isSafeToExtendWorkspace(path.node.declaration as CallExpression)
+            ) {
+              isValidVitestConfig = true;
+            }
+          },
+        });
+        if (!isValidVitestConfig) {
+          reasons.push(`Found an invalid Vitest config file: ${vitestConfigFile}`);
+        }
+      } catch {
+        reasons.push(`Error reading/parsing Vitest config file: ${vitestConfigFile}`);
+      }
+    }
📝 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.

Suggested change
const vitestConfigFile = find.any(
['ts', 'js', 'tsx', 'jsx', 'cts', 'cjs', 'mts', 'mjs'].map((ex) => `vitest.config.${ex}`),
{ cwd: state.directory, last: projectRoot }
);
if (vitestConfigFile?.endsWith('.cts') || vitestConfigFile?.endsWith('.cjs')) {
reasons.push(`Cannot auto-update CommonJS config file: ${vitestConfigFile}`);
} else if (vitestConfigFile) {
let isValidVitestConfig = false;
const configContent = await fs.readFile(vitestConfigFile, 'utf8');
const parsedConfig = babel.babelParse(configContent);
babel.traverse(parsedConfig, {
ExportDefaultDeclaration(path) {
if (
isDefineConfigExpression(path.node.declaration) &&
isSafeToExtendWorkspace(path.node.declaration as CallExpression)
) {
isValidVitestConfig = true;
}
},
});
if (!isValidVitestConfig) {
reasons.push(`Found an invalid Vitest config file: ${vitestConfigFile}`);
}
return reasons.length
? { type: CompatibilityType.INCOMPATIBLE, reasons }
: { type: CompatibilityType.COMPATIBLE };
}
const vitestConfigFile = find.any(
['ts', 'js', 'tsx', 'jsx', 'cts', 'cjs', 'mts', 'mjs'].map((ex) => `vitest.config.${ex}`),
{ cwd: state.directory, last: projectRoot }
);
if (vitestConfigFile?.endsWith('.cts') || vitestConfigFile?.endsWith('.cjs')) {
reasons.push(`Cannot auto-update CommonJS config file: ${vitestConfigFile}`);
} else if (vitestConfigFile) {
try {
let isValidVitestConfig = false;
const configContent = await fs.readFile(vitestConfigFile, 'utf8');
const parsedConfig = babel.babelParse(configContent);
babel.traverse(parsedConfig, {
ExportDefaultDeclaration(path) {
if (
isDefineConfigExpression(path.node.declaration) &&
isSafeToExtendWorkspace(path.node.declaration as CallExpression)
) {
isValidVitestConfig = true;
}
},
});
if (!isValidVitestConfig) {
reasons.push(`Found an invalid Vitest config file: ${vitestConfigFile}`);
}
} catch {
reasons.push(`Error reading/parsing Vitest config file: ${vitestConfigFile}`);
}
}

return {
type: CompatibilityType.INCOMPATIBLE,
reasons: deps
.filter((p) => !context[p as keyof typeof context])
.map((p) => `Missing ${p} on context`),
};

return reasons.length
? { type: CompatibilityType.INCOMPATIBLE, reasons }
: { type: CompatibilityType.COMPATIBLE };
},
};