Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ it('builds a real payload through the TypeScript-backed analyzer', async () => {
name: 'label',
table: { category: 'inputs' },
});
expect(payload?.angularComponentMeta?.name).toBe('ButtonComponent');
expect(payload?.angularComponentMeta?.entry.name).toBe('ButtonComponent');
} finally {
manager.dispose();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ describe('buildDocgenPayload', () => {
name: 'label',
table: { category: 'inputs', defaultValue: { summary: 'Click me' } },
});
expect(payload?.angularComponentMeta).toBe(classMeta);
expect(payload?.angularComponentMeta?.entry).toBe(classMeta);
expect(payload?.compodoc).toBeUndefined();
expect(payload?.subcomponents).toBeUndefined();
expect(payload?.error).toBeUndefined();
Expand Down
11 changes: 8 additions & 3 deletions code/frameworks/angular-vite/src/docgen/build-docgen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import type {

import { resolve } from 'node:path';

import type { EnumType } from '@storybook/angular-compodoc';
import type {
AngularClassMeta,
AngularComponentMetaResult,
Expand All @@ -21,9 +22,13 @@ export interface AngularDocgenOptions {
angularFilterNonInputControls?: boolean;
}

export interface AngularComponentSnippetMeta {
entry: AngularClassMeta;
enums: EnumType[];
}

export type AngularDocgenPayload = DocgenPayload & {
// The analyzer's record for the class, not filtered by `angularFilterNonInputControls`.
angularComponentMeta?: AngularClassMeta;
angularComponentMeta?: AngularComponentSnippetMeta;
};

// Structural on purpose: tests hand in a stub instead of a real TypeScript-backed analyzer.
Expand Down Expand Up @@ -150,6 +155,6 @@ export const buildDocgenPayload = (
summary: jsDocTags.summary?.[0],
jsDocTags,
argTypes,
angularComponentMeta: meta.entry,
angularComponentMeta: { entry: meta.entry, enums: meta.json.miscellaneous?.enumerations ?? [] },
};
};
54 changes: 42 additions & 12 deletions code/frameworks/angular-vite/src/docgen/story-docs-build.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';

import { vol } from 'memfs';

import type { AngularComponentMetaSource } from './build-docgen.ts';
import type { AngularDocgenPayload } from './build-docgen.ts';
import { buildStoryDocsPayload } from './story-docs-build.ts';

vi.mock('node:fs', { spy: true });
Expand Down Expand Up @@ -42,7 +42,7 @@ const givenStoryFile = (source: string) => {
};

describe('buildStoryDocsPayload', () => {
it('returns undefined for entries without a story file or with an unparsable one', () => {
it('returns undefined for entries without a story file or with an unparsable one', async () => {
const docsEntry: IndexEntry = {
id: 'docs--page',
name: 'Page',
Expand All @@ -52,20 +52,22 @@ describe('buildStoryDocsPayload', () => {
storiesImports: [],
tags: [],
};
expect(buildStoryDocsPayload({ entry: docsEntry }, { manager: undefined })).toBeUndefined();
expect(
await buildStoryDocsPayload({ entry: docsEntry }, { getDocgenPayload: undefined })
).toBeUndefined();

givenStoryFile('export default { title: "Broken" ');
expect(buildStoryDocsPayload({ entry }, { manager: undefined })).toBeUndefined();
expect(await buildStoryDocsPayload({ entry }, { getDocgenPayload: undefined })).toBeUndefined();
});

it('still emits description-only stories when the analyzer is unavailable', () => {
it('still emits description-only stories when core/docgen is unavailable', async () => {
givenStoryFile(`
export default { title: 'Example/Button' };
/** Documented without a component. */
export const Default = {};
`);

const payload = buildStoryDocsPayload({ entry }, { manager: undefined });
const payload = await buildStoryDocsPayload({ entry }, { getDocgenPayload: undefined });

expect(payload?.name).toBe('Button');
expect(Object.values(payload!.stories)[0]).toEqual({
Expand All @@ -75,19 +77,47 @@ describe('buildStoryDocsPayload', () => {
});
});

it('drops the snippet without erroring when the analyzer throws', () => {
it('builds a snippet from the raw analyzer fields core/docgen carries alongside argTypes', async () => {
givenStoryFile(`
import { ButtonComponent } from './button.component';
export default { title: 'Example/Button', component: ButtonComponent };
export const Default = { args: { label: 'Save' } };
`);
const manager: AngularComponentMetaSource = {
extractComponentMeta: () => {
throw new Error('ts blew up');
},
const getDocgenPayload = async (): Promise<AngularDocgenPayload> =>
({
id: 'example-button',
name: 'ButtonComponent',
path: STORY_PATH,
jsDocTags: {},
angularComponentMeta: {
entry: {
name: 'ButtonComponent',
selector: 'sb-button',
inputsClass: [{ name: 'label' }],
outputsClass: [],
},
enums: [],
},
}) as AngularDocgenPayload;
Comment on lines +86 to +101

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.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Use the required Vitest spy setup for getDocgenPayload.

Declare the resolver spy before the test cases. Configure its behavior in beforeEach. Use vi.mocked() to access the typed mock.

  • code/frameworks/angular-vite/src/docgen/story-docs-build.test.ts#L86-L99: Configure the successful payload response in beforeEach.
  • code/frameworks/angular-vite/src/docgen/story-docs-build.test.ts#L114-L116: Configure the rejected query response in beforeEach.
  • code/lib/docgen-harness/src/angular/angular-story-docs-snippets.test.ts#L37-L63: Replace the direct resolver callback with the shared typed spy.
  • code/lib/docgen-harness/src/angular/story-docs/build-story-docs.test.ts#L26-L52: Replace the direct resolver callback with the shared typed spy.

As per coding guidelines, implement mock behaviors in beforeEach blocks, use vi.mocked() to type and access mocked functions, and avoid direct function mocking without vi.mocked().

📍 Affects 3 files
  • code/frameworks/angular-vite/src/docgen/story-docs-build.test.ts#L86-L99 (this comment)
  • code/frameworks/angular-vite/src/docgen/story-docs-build.test.ts#L114-L116
  • code/lib/docgen-harness/src/angular/angular-story-docs-snippets.test.ts#L37-L63
  • code/lib/docgen-harness/src/angular/story-docs/build-story-docs.test.ts#L26-L52
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@code/frameworks/angular-vite/src/docgen/story-docs-build.test.ts` around
lines 86 - 99, Replace direct getDocgenPayload resolver implementations with a
shared resolver spy declared before tests, and configure each mock behavior in
beforeEach using vi.mocked(). In
code/frameworks/angular-vite/src/docgen/story-docs-build.test.ts#L86-L99,
configure the successful payload; at `#L114-L116`, configure the rejected query;
in
code/lib/docgen-harness/src/angular/angular-story-docs-snippets.test.ts#L37-L63
and
code/lib/docgen-harness/src/angular/story-docs/build-story-docs.test.ts#L26-L52,
use the shared typed spy instead of direct resolver callbacks.

Source: Coding guidelines


const payload = await buildStoryDocsPayload({ entry }, { getDocgenPayload });

const story = Object.values(payload!.stories)[0];
expect(story.snippet).toContain('sb-button');
expect(story.snippet).toContain(`[label]="'Save'"`);
});

it('drops the snippet without erroring when querying core/docgen throws', async () => {
givenStoryFile(`
import { ButtonComponent } from './button.component';
export default { title: 'Example/Button', component: ButtonComponent };
export const Default = { args: { label: 'Save' } };
`);
const getDocgenPayload = async (): Promise<AngularDocgenPayload | undefined> => {
throw new Error('core/docgen query failed');
};

const payload = buildStoryDocsPayload({ entry }, { manager });
const payload = await buildStoryDocsPayload({ entry }, { getDocgenPayload });

const story = Object.values(payload!.stories)[0];
expect(story.snippet).toBeUndefined();
Expand Down
41 changes: 20 additions & 21 deletions code/frameworks/angular-vite/src/docgen/story-docs-build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import type { StoryDoc, StoryDocsPayload, StoryDocsProviderInput } from 'storybo
import { resolve } from 'node:path';

import type { EnumType, Property } from '@storybook/angular-compodoc';
import type { AngularClassMeta, AngularComponentMetaResult } from '@storybook/angular-cm';
import type { AngularComponentMetaSource } from './build-docgen.ts';
import type { AngularClassMeta } from '@storybook/angular-cm';
import type { AngularComponentSnippetMeta, AngularDocgenPayload } from './build-docgen.ts';
import { parseStoryFile, resolveComponentOf } from './resolve-component.ts';
import { buildComponentOutletTemplate } from '../template-grammar.ts';
import {
Expand All @@ -23,17 +23,18 @@ import {
} from './story-docs-snippet.ts';

export interface BuildStoryDocsContext {
// `undefined` when the analyzer could not be created; descriptions still extract without it.
manager: AngularComponentMetaSource | undefined;
getDocgenPayload:
| ((componentId: string) => Promise<AngularDocgenPayload | undefined>)
| undefined;
resolvePath?: (importPath: string) => string;
}

// Stories that declare their own `render` get no snippet: their template is a runtime value static
// analysis cannot see, and a component-derived one would misrepresent it.
export const buildStoryDocsPayload = (
export const buildStoryDocsPayload = async (
input: StoryDocsProviderInput,
context: BuildStoryDocsContext
): StoryDocsPayload | undefined => {
): Promise<StoryDocsPayload | undefined> => {
const storyImportPath = getStoryImportPathFromEntry(input.entry);
if (!storyImportPath) {
return undefined;
Expand All @@ -51,18 +52,16 @@ export const buildStoryDocsPayload = (
const resolution = resolveComponentOf(csf, storyPath);
const component = 'reason' in resolution ? undefined : resolution.component;

let meta: AngularComponentMetaResult | undefined;
if (component?.path && context.manager) {
let snippetMeta: AngularComponentSnippetMeta | undefined;
if (component?.path && context.getDocgenPayload) {
try {
meta = context.manager.extractComponentMeta(component.path, {
exportName: component.exportName,
localName: component.localName,
});
const docgenPayload = await context.getDocgenPayload(getComponentIdFromEntry(input.entry));
snippetMeta = docgenPayload?.angularComponentMeta;
} catch {
meta = undefined;
snippetMeta = undefined;
}
}
const snippetContext = meta ? createSnippetContext(meta) : undefined;
const snippetContext = snippetMeta ? createSnippetContext(snippetMeta) : undefined;

const displayName =
component && (component.exportName === 'default' ? component.localName : component.exportName);
Expand Down Expand Up @@ -103,7 +102,7 @@ export const buildStoryDocsPayload = (
return {
id: getComponentIdFromEntry(input.entry),
// The analyzer knows the class name even when the story file imported it as a default export.
name: meta?.entry.name ?? displayName ?? titleName,
name: snippetMeta?.entry.name ?? displayName ?? titleName,
path: storyImportPath,
stories,
};
Expand All @@ -124,22 +123,22 @@ const inputsOf = (entry: AngularClassMeta): Property[] =>
const outputsOf = (entry: AngularClassMeta): Property[] =>
'outputsClass' in entry ? (entry.outputsClass ?? []) : [];

const createSnippetContext = (meta: AngularComponentMetaResult): SnippetContext => {
const inputNames = new Set(inputsOf(meta.entry).map((input) => input.name));
const createSnippetContext = ({ entry, enums }: AngularComponentSnippetMeta): SnippetContext => {
const inputNames = new Set(inputsOf(entry).map((input) => input.name));
const outputs: string[] = [];
for (const output of outputsOf(meta.entry)) {
for (const output of outputsOf(entry)) {
// model() lands under the same bare name in both arrays; its output binds as `${name}Change`.
const bindingName = inputNames.has(output.name) ? `${output.name}Change` : output.name;
if (!outputs.includes(bindingName)) {
outputs.push(bindingName);
}
}
return {
selector: meta.entry.selector,
componentName: meta.entry.name,
selector: entry.selector,
componentName: entry.name,
inputNames,
outputs,
enums: meta.json.miscellaneous?.enumerations ?? [],
enums,
};
};

Expand Down
36 changes: 19 additions & 17 deletions code/frameworks/angular-vite/src/docgen/story-docs-preset.ts
Original file line number Diff line number Diff line change
@@ -1,39 +1,41 @@
import { STORY_FILE_TEST_REGEXP, getStoryImportPathFromEntry } from 'storybook/internal/common';
import { getService } from 'storybook/internal/core-server';
import { logger } from 'storybook/internal/node-logger';
import type { StoryDocsProviderPreset } from 'storybook/internal/types';

import { AngularComponentMetaManager } from '@storybook/angular-cm';
import type { AngularDocgenPayload } from './build-docgen.ts';
import { buildStoryDocsPayload } from './story-docs-build.ts';

const createManager = async (): Promise<AngularComponentMetaManager | undefined> => {
// `core/docgen` is only registered when `experimentalDocgenServer` set up its worker (see
// `common-preset.ts`); both services are gated by the same feature, but registration order isn't
// a type-level guarantee, so this stays defensive rather than asserting the service exists.
Comment on lines +9 to +11

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.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Remove the cross-file source reference.

Keep the registration-order rationale. Remove the common-preset.ts reference from this comment.

As per coding guidelines, comments should explain maintenance-relevant rationale, not cross-file line references.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@code/frameworks/angular-vite/src/docgen/story-docs-preset.ts` around lines 9
- 11, Update the comment near the `core/docgen` registration to retain the
rationale for defensively handling registration order and avoiding a type
assertion, but remove the explicit `common-preset.ts` cross-file reference.

Source: Coding guidelines

let warnedMissingDocgenService = false;

const getDocgenPayload = async (componentId: string): Promise<AngularDocgenPayload | undefined> => {
try {
const typescript = await import('typescript');
return new AngularComponentMetaManager(typescript.default ?? typescript);
const docgenService = getService('core/docgen', { internal: true });
return await docgenService.queries.docgen.loaded({ id: componentId });
} catch (error) {
logger.warn(
`Angular story snippets are unavailable: the component meta analyzer could not be created. ${
error instanceof Error ? error.message : String(error)
}`
);
if (!warnedMissingDocgenService) {
warnedMissingDocgenService = true;
logger.warn(
`Angular story snippets are unavailable: querying core/docgen failed. ${
error instanceof Error ? error.message : String(error)
}`
);
}
return undefined;
}
};

export const experimental_storyDocsProvider: StoryDocsProviderPreset = async (nextStoryDocs) => {
// Scoped to the composed chain rather than the module, so the manager has one owner and one
// lifetime.
let managerPromise: Promise<AngularComponentMetaManager | undefined> | undefined;

return async (input) => {
const storyImportPath = getStoryImportPathFromEntry(input.entry);
if (!storyImportPath || !STORY_FILE_TEST_REGEXP.test(storyImportPath)) {
return nextStoryDocs(input);
}

const manager = await (managerPromise ??= createManager());
const ours = buildStoryDocsPayload(input, { manager });
// The language service holds large type caches; check heap pressure after each extraction.
manager?.recycleIfHeapPressured();
const ours = await buildStoryDocsPayload(input, { getDocgenPayload });

if (!ours) {
return nextStoryDocs(input);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,12 @@ import { loadCsf } from 'storybook/internal/csf-tools';
import type { IndexEntry } from 'storybook/internal/types';

import { AngularComponentMetaManager } from '@storybook/angular-cm';
import type { AngularDocgenPayload } from '../../../../frameworks/angular-vite/src/docgen/build-docgen.ts';
import { buildStoryDocsPayload } from '../../../../frameworks/angular-vite/src/docgen/story-docs-build.ts';
import {
parseStoryFile,
resolveComponentOf,
} from '../../../../frameworks/angular-vite/src/docgen/resolve-component.ts';
import {
expectNoStaleSnippets,
fixtureCases,
Expand All @@ -24,6 +29,41 @@ import {
// component file resolves to its own per-fixture project.
const manager = new AngularComponentMetaManager(ts);

// In production `getDocgenPayload` queries the already-registered `core/docgen` service, which is
// keyed by component id and resolves the story's component internally. There is no service here,
// so this stands in for it: resolve the same component `buildStoryDocsPayload` would (the fixture
// carries exactly one), then extract straight off the manager, matching what `buildDocgenPayload`
// stores on `AngularDocgenPayload` in production.
const buildGetDocgenPayload =
(storyPath: string, title: string) => async (): Promise<AngularDocgenPayload | undefined> => {
const parsed = parseStoryFile(storyPath, title);
if (!parsed) {
return undefined;
}
const resolution = resolveComponentOf(parsed.csf, storyPath);
const component = 'reason' in resolution ? undefined : resolution.component;
if (!component?.path) {
return undefined;
}
const meta = manager.extractComponentMeta(component.path, {
exportName: component.exportName,
localName: component.localName,
});
if (!meta) {
return undefined;
}
return {
id: 'fixture',
name: meta.entry.name,
path: storyPath,
jsDocTags: {},
angularComponentMeta: {
entry: meta.entry,
enums: meta.json.miscellaneous?.enumerations ?? [],
},
};
};

afterAll(() => {
manager.dispose();
});
Expand All @@ -47,7 +87,10 @@ describe('angular story-docs server snippets', () => {
subtype: 'story',
importPath: storyPath,
};
const payload = buildStoryDocsPayload({ entry }, { manager, resolvePath: (path) => path });
const payload = await buildStoryDocsPayload(
{ entry },
{ getDocgenPayload: buildGetDocgenPayload(storyPath, title), resolvePath: (path) => path }
);
expect(payload).toBeDefined();

for (const [exportName, story] of storyExports) {
Expand Down
Loading