Skip to content
Closed
Show file tree
Hide file tree
Changes from 4 commits
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
46 changes: 43 additions & 3 deletions code/core/src/core-server/manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -517,15 +517,23 @@ export function renderManifestComponentsPage(manifest: ComponentsManifest) {
.card > .tg-err:checked ~ .panels .panel-err {
display: grid;
}

.card > .tg-warn:checked ~ .panels .panel-warn {
display: grid;
}

.card > .tg-stories:checked ~ .panels .panel-stories {
display: grid;
}
Comment on lines +520 to 527
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

Remove or complete empty CSS rule blocks.

These CSS rule blocks appear to be incomplete or placeholder code. They should either be removed or completed with the intended styles.

-      
-      .card > .tg-warn:checked ~ .panels .panel-warn {
-          display: grid;
-      }
-      
-      .card > .tg-stories:checked ~ .panels .panel-stories {
-          display: grid;
-      }
📝 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
.card > .tg-warn:checked ~ .panels .panel-warn {
display: grid;
}
.card > .tg-stories:checked ~ .panels .panel-stories {
display: grid;
}
🤖 Prompt for AI Agents
In code/core/src/core-server/manifest.ts around lines 520 to 527, there are two
CSS rule blocks that look like incomplete placeholders (.card > .tg-warn:checked
~ .panels .panel-warn and .card > .tg-stories:checked ~ .panels .panel-stories);
either remove these rules entirely if they are not needed, or complete them with
the intended panel styles (for example set grid-template-columns/rows, gap,
align-items, padding, and any show/hide behavior) so they’re not empty
placeholders — choose one of those two options and update the file accordingly.


/* Add vertical spacing around panels only when any panel is visible */
.card > .tg-err:checked ~ .panels,
.card > .tg-warn:checked ~ .panels,
.card > .tg-stories:checked ~ .panels,
.card > .tg-props:checked ~ .panels {
margin: 10px 0;
}

/* Optional: a subtle 1px ring on the active badge, using :has() if available */
@supports selector(.card:has(.tg-err:checked)) {
.card:has(.tg-err:checked) label[for$='-err'],
Expand All @@ -536,6 +544,25 @@ export function renderManifestComponentsPage(manifest: ComponentsManifest) {
border-color: currentColor;
}
}

/* Wrap long lines in code blocks at ~120 characters */
pre, code {
font-family: ui-monospace, SFMono-Regular, Menlo, Monaco, Consolas, "Liberation Mono", "Courier New", monospace;
}
pre {
white-space: pre-wrap;
overflow-wrap: anywhere;
word-break: break-word;
overflow-x: auto; /* fallback for extremely long tokens */
margin: 8px 0 0;
}
pre > code {
display: block;
white-space: inherit;
overflow-wrap: inherit;
word-break: inherit;
inline-size: min(100%, 120ch);
}
</style>
</head>
<body>
Expand Down Expand Up @@ -752,9 +779,22 @@ function renderComponentCard(key: string, c: ComponentManifest, id: string) {
</div>`
)
.join('')}


${
c.import
? `<div class="note ok">
<div class="row">
<span class="ex-name">Imports</span>
</div>
<pre><code>${c.import}</code></pre>
</div>`
: ''
}
Comment on lines +784 to +795
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 | 🔴 Critical

Escape import content to prevent XSS.

The import string c.import is rendered without escaping, which could lead to XSS vulnerabilities if the import content contains malicious HTML/script tags.

Apply the esc() function:

           ${
             c.import
               ? `<div class="note ok">
                 <div class="row">
                   <span class="ex-name">Imports</span>
                 </div>
-                <pre><code>${c.import}</code></pre>
+                <pre><code>${esc(c.import)}</code></pre>
               </div>`
               : ''
           }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In code/core/src/core-server/manifest.ts around lines 782 to 793, the template
renders c.import unescaped which can lead to XSS; replace the raw interpolation
with an escaped value (e.g., use esc(c.import) or esc(String(c.import))) when
inserting into the <pre><code> block, and ensure the esc function is
imported/available in this module; keep the conditional logic the same but wrap
the displayed content in esc(...) to prevent HTML/script injection.


${okStories
.map(
(ex, k) => `
(ex) => `
<div class="note ok">
<div class="row">
<span class="ex-name">${esc(ex.name)}</span>
Expand Down
1 change: 1 addition & 0 deletions code/renderers/react/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@
"acorn-walk": "^7.2.0",
"babel-plugin-react-docgen": "^4.2.1",
"comment-parser": "^1.4.1",
"empathic": "^2.0.0",
"es-toolkit": "^1.36.0",
"escodegen": "^2.1.0",
"expect-type": "^0.15.0",
Expand Down
40 changes: 30 additions & 10 deletions code/renderers/react/src/componentManifest/generator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,26 @@ import { dedent } from 'ts-dedent';

import { componentManifestGenerator } from './generator';

vi.mock('storybook/internal/common', async (importOriginal) => {
return {
...(await importOriginal()),
JsPackageManagerFactory: {
getPackageManager: () => ({
primaryPackageJson: {
packageJson: {
name: 'some-package',
},
},
}),
},
};
});
vi.mock('node:fs/promises', async () => (await import('memfs')).fs.promises);
vi.mock('node:fs', async () => (await import('memfs')).fs);

vi.mock('empathic/find', async () => ({
up: (path: string) => '/app/package.json',
}));
Comment on lines +33 to +35
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

Mock empathic/find without spy: true option.

According to the coding guidelines, all package and file mocks in Vitest tests should use vi.mock() with the spy: true option.

As per coding guidelines

Apply this diff:

-vi.mock('empathic/find', async () => ({
+vi.mock('empathic/find', { spy: true }, async () => ({
   up: (path: string) => '/app/package.json',
 }));
📝 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('empathic/find', async () => ({
up: (path: string) => '/app/package.json',
}));
vi.mock('empathic/find', async () => ({
up: (path: string) => '/app/package.json',
}), { spy: true });
🤖 Prompt for AI Agents
In code/renderers/react/src/componentManifest/generator.test.ts around lines
27-29, the Vitest mock for 'empathic/find' is missing the required spy option;
update the vi.mock call to pass the options object with { spy: true } as the
third argument while keeping the async factory intact (i.e.,
vi.mock('empathic/find', async () => ({ up: (path: string) =>
'/app/package.json' }), { spy: true })).

vi.mock('tsconfig-paths', () => ({ loadConfig: () => ({ resultType: null!, message: null! }) }));

// Use the provided indexJson from this file
Expand Down Expand Up @@ -97,6 +115,7 @@ beforeEach(() => {
vi.spyOn(process, 'cwd').mockReturnValue('/app');
vol.fromJSON(
{
['./package.json']: JSON.stringify({ name: 'some-package' }),
['./src/stories/Button.stories.ts']: dedent`
import type { Meta, StoryObj } from '@storybook/react';
import { fn } from 'storybook/test';
Expand Down Expand Up @@ -205,8 +224,8 @@ beforeEach(() => {
});

test('componentManifestGenerator generates correct id, name, description and examples ', async () => {
const generator = await componentManifestGenerator();
const manifest = await generator({
const generator = await componentManifestGenerator(undefined, { configDir: '.storybook' } as any);
const manifest = await generator?.({
getIndex: async () => indexJson,
} as unknown as StoryIndexGenerator);

Expand All @@ -217,7 +236,7 @@ test('componentManifestGenerator generates correct id, name, description and exa
"description": "Primary UI component for user interaction",
"error": undefined,
"id": "example-button",
"import": undefined,
"import": "import { Button } from "some-package";",
"jsDocTags": {},
"name": "Button",
"path": "./src/stories/Button.stories.ts",
Expand Down Expand Up @@ -321,7 +340,7 @@ test('componentManifestGenerator generates correct id, name, description and exa
"description": "Description from meta and very long.",
"error": undefined,
"id": "example-header",
"import": "import { Header } from '@design-system/components/Header';",
"import": "import { Header } from "some-package";",
"jsDocTags": {
"import": [
"import { Header } from '@design-system/components/Header';",
Expand Down Expand Up @@ -418,6 +437,7 @@ test('componentManifestGenerator generates correct id, name, description and exa
async function getManifestForStory(code: string) {
vol.fromJSON(
{
['./package.json']: JSON.stringify({ name: 'some-package' }),
['./src/stories/Button.stories.ts']: code,
['./src/stories/Button.tsx']: dedent`
import React from 'react';
Expand All @@ -441,7 +461,7 @@ async function getManifestForStory(code: string) {
'/app'
);

const generator = await componentManifestGenerator();
const generator = await componentManifestGenerator(undefined, { configDir: '.storybook' } as any);
const indexJson = {
v: 5,
entries: {
Expand All @@ -459,11 +479,11 @@ async function getManifestForStory(code: string) {
},
};

const manifest = await generator({
const manifest = await generator?.({
getIndex: async () => indexJson,
} as unknown as StoryIndexGenerator);

return manifest.components['example-button'];
return manifest?.components?.['example-button'];
}

function withCSF3(body: string) {
Expand Down Expand Up @@ -497,7 +517,7 @@ test('fall back to index title when no component name', async () => {
"description": "Primary UI component for user interaction",
"error": undefined,
"id": "example-button",
"import": undefined,
"import": "import { Button } from "some-package";",
"jsDocTags": {},
"name": "Button",
"path": "./src/stories/Button.stories.ts",
Expand Down Expand Up @@ -542,7 +562,7 @@ test('component exported from other file', async () => {
"description": "Primary UI component for user interaction",
"error": undefined,
"id": "example-button",
"import": undefined,
"import": "import { Button } from "some-package";",
"jsDocTags": {},
"name": "Button",
"path": "./src/stories/Button.stories.ts",
Expand Down Expand Up @@ -594,7 +614,7 @@ test('unknown expressions', async () => {
"description": "Primary UI component for user interaction",
"error": undefined,
"id": "example-button",
"import": undefined,
"import": "import { Button } from "some-package";",
"jsDocTags": {},
"name": "Button",
"path": "./src/stories/Button.stories.ts",
Expand Down
24 changes: 20 additions & 4 deletions code/renderers/react/src/componentManifest/generator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,14 @@ import { readFile } from 'node:fs/promises';
import { recast } from 'storybook/internal/babel';
import { loadCsf } from 'storybook/internal/csf-tools';
import { extractDescription } from 'storybook/internal/csf-tools';
import { type ComponentManifestGenerator } from 'storybook/internal/types';
import { type ComponentManifestGenerator, type PresetPropertyFn } from 'storybook/internal/types';
import { type ComponentManifest } from 'storybook/internal/types';

import * as find from 'empathic/find';
import path from 'pathe';

import { getCodeSnippet } from './generateCodeSnippet';
import { getComponentImports } from './getComponentImports';
import { extractJSDocInfo } from './jsdocTags';
import { type DocObj, getMatchingDocgen, parseWithReactDocgen } from './reactDocgen';
import { groupBy, invariant } from './utils';
Expand All @@ -17,7 +19,9 @@ interface ReactComponentManifest extends ComponentManifest {
reactDocgen?: DocObj;
}

export const componentManifestGenerator = async () => {
export const componentManifestGenerator: PresetPropertyFn<
'experimental_componentManifestGenerator'
> = async (config, options) => {
return (async (storyIndexGenerator) => {
const index = await storyIndexGenerator.getIndex();

Expand All @@ -32,7 +36,8 @@ export const componentManifestGenerator = async () => {
);
const components = await Promise.all(
singleEntryPerComponent.flatMap(async (entry): Promise<ReactComponentManifest> => {
const storyFile = await readFile(path.join(process.cwd(), entry.importPath), 'utf-8');
const storyAbsPath = path.join(process.cwd(), entry.importPath);
const storyFile = await readFile(storyAbsPath, 'utf-8');
const csf = loadCsf(storyFile, { makeTitle: (title) => title ?? 'No title' }).parse();
const name = csf._meta?.component ?? entry.title.split('/').at(-1)!;
const id = entry.id.split('--')[0];
Expand All @@ -55,11 +60,23 @@ export const componentManifestGenerator = async () => {
})
.filter(Boolean);

const nearestPkg = find.up('package.json', {
cwd: path.dirname(storyAbsPath),
last: process.cwd(),
});
const packageName = nearestPkg
? JSON.parse(await readFile(nearestPkg, 'utf-8')).name
: undefined;
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

Add error handling for package.json parsing.

The JSON.parse call on Line 68 could throw if the package.json file is malformed. Consider wrapping it in a try-catch block to prevent uncaught exceptions.

         const nearestPkg = find.up('package.json', {
           cwd: path.dirname(storyAbsPath),
           last: process.cwd(),
         });
-        const packageName = nearestPkg
-          ? JSON.parse(await readFile(nearestPkg, 'utf-8')).name
-          : undefined;
+        let packageName: string | undefined;
+        if (nearestPkg) {
+          try {
+            packageName = JSON.parse(await readFile(nearestPkg, 'utf-8')).name;
+          } catch (e) {
+            // Skip if package.json is malformed
+            packageName = undefined;
+          }
+        }
🤖 Prompt for AI Agents
In code/renderers/react/src/componentManifest/generator.ts around lines 63–69,
the direct JSON.parse of the nearest package.json can throw on malformed JSON;
wrap the parse in a try/catch so a bad package.json does not crash the process —
read the file as before, then attempt JSON.parse inside a try block, on success
set packageName to the parsed.name, on error catch and set packageName =
undefined (and optionally log a warning with the file path and error) so
execution continues safely.

const fallbackImport = packageName ? `import { ${name} } from "${packageName}";` : '';
const calculatedImports =
getComponentImports(csf, packageName).imports.join('\n').trim() ?? fallbackImport;

const base = {
id,
name,
path: importPath,
stories,
import: calculatedImports,
jsDocTags: {},
} satisfies Partial<ComponentManifest>;

Expand Down Expand Up @@ -133,7 +150,6 @@ export const componentManifestGenerator = async () => {
name,
description: manifestDescription?.trim(),
summary: tags.summary?.[0],
import: tags.import?.[0],
reactDocgen: docgen,
jsDocTags: tags,
stories,
Expand Down
Loading
Loading