diff --git a/MIGRATION.md b/MIGRATION.md index c835f7adea80..293c43b940fe 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -530,6 +530,35 @@ ## From version 10.5.x to 10.6.0 +### Angular Vite: a new `propsTable` framework option + +`@storybook/angular-vite` now lets you choose which members the props table documents, through a `propsTable` framework option. It defaults to `'api'`, which leaves out TypeScript `private` properties and methods, ECMAScript private `#` members, and anything tagged `@internal`. No template can reach a `private` property or method, and `@internal` declares a member non-API, so a row for them documents your component's wiring rather than its API. Injected services such as `private readonly cdr = inject(ChangeDetectorRef)` are the common case. + +Declared inputs and outputs are always documented, whatever their TypeScript visibility. Angular only honors access modifiers on input bindings behind the opt-in `strictInputAccessModifiers` compiler flag and never checks them on output bindings, so even a `private` input or output is API a parent template can bind. + +`protected` members are documented. Angular templates have been able to bind them since Angular 14, so they are real API. + +The default only changes what you see when `features.experimentalDocgenServer` is on. With the Compodoc pipeline, which is still the default, Storybook cannot interpret Compodoc's visibility data reliably and the props table is unchanged. + +Set the option to `'all'` to document every member: + +```ts +// .storybook/main.ts +framework: { + name: '@storybook/angular-vite', + options: { + // 'all' documents every member. + // 'api' (the default) leaves out private and `#` properties and methods, and @internal members. + // 'inputs' documents the inputs section only. + propsTable: 'all', + }, +}, +``` + +To drop a single member the default keeps, tag it `@ignore`. + +`features.angularFilterNonInputControls` is deprecated on `@storybook/angular-vite` and will be removed in Storybook 11: `true` maps to `propsTable: 'inputs'` and `false` to `propsTable: 'all'`. Setting both leaves `propsTable` in charge. `@storybook/angular` (webpack) is unaffected and keeps reading the feature. + ### MCP tool names follow toolset.method Storybook's MCP tools are now named from their toolset and method (`stories.preview` → `stories-preview`). Update agent prompts, skills, and hard-coded tool allowlists: diff --git a/code/core/src/types/modules/core-common.ts b/code/core/src/types/modules/core-common.ts index 13effa7b9caa..8089aa08a6fe 100644 --- a/code/core/src/types/modules/core-common.ts +++ b/code/core/src/types/modules/core-common.ts @@ -603,7 +603,12 @@ export interface StorybookFeatures { * Set NODE_ENV to development in built Storybooks for better testability and debuggability */ developmentModeForBuild?: boolean; - /** Only show input controls in Angular */ + /** + * Only show input controls in Angular. + * + * @deprecated On `@storybook/angular-vite`, use the `propsTable` framework option instead: + * `'inputs'` for this flag on, `'all'` for it off. Still read by `@storybook/angular`. + */ angularFilterNonInputControls?: boolean; /** diff --git a/code/frameworks/angular-vite/src/client/compodoc.test.ts b/code/frameworks/angular-vite/src/client/compodoc.test.ts index a9f8ad935713..b2ffaef6ee91 100644 --- a/code/frameworks/angular-vite/src/client/compodoc.test.ts +++ b/code/frameworks/angular-vite/src/client/compodoc.test.ts @@ -1,136 +1,65 @@ -import { describe, expect, it } from 'vitest'; +// How this framework's `propsTable` mode reaches the legacy Compodoc adapter. The module reads +// `STORYBOOK_ANGULAR_OPTIONS` at evaluation time, so every case re-imports it fresh. +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { extractType, setCompodocJson } from './compodoc.ts'; -import type { CompodocJson, Decorator } from './compodoc-types.ts'; +import type { CompodocJson, Directive } from './compodoc-types.ts'; -const makeProperty = (compodocType?: string) => ({ - type: compodocType, - name: 'dummy', - decorators: [] as Decorator[], - optional: true, -}); +const compodocJson: Partial = { + components: [], + directives: [], + pipes: [], + injectables: [], + classes: [], + miscellaneous: { typealiases: [], enumerations: [] } as never, +}; + +const componentData: Partial = { + name: 'ProbeComponent', + type: 'component', + inputsClass: [{ name: 'label', type: 'string', optional: false }], + outputsClass: [], + propertiesClass: [{ name: 'note', type: 'string', optional: false }], + methodsClass: [], +}; -const getDummyCompodocJson = () => { - return { - miscellaneous: { - typealiases: [ - { - name: 'EnumAlias', - ctype: 'miscellaneous', - subtype: 'typealias', - rawtype: 'EnumNumeric', - file: 'src/stories/component-with-enums/enums.component.ts', - description: '', - kind: 161, - }, - { - name: 'TypeAlias', - ctype: 'miscellaneous', - subtype: 'typealias', - rawtype: '"Type Alias 1" | "Type Alias 2" | "Type Alias 3"', - file: 'src/stories/component-with-enums/enums.component.ts', - description: '', - kind: 168, - }, - ], - enumerations: [ - { - name: 'EnumNumeric', - childs: [ - { - name: 'FIRST', - }, - { - name: 'SECOND', - }, - { - name: 'THIRD', - }, - ], - ctype: 'miscellaneous', - subtype: 'enum', - description: '

Button Priority

\n', - file: 'src/stories/component-with-enums/enums.component.ts', - }, - { - name: 'EnumNumericInitial', - childs: [ - { - name: 'UNO', - value: '1', - }, - { - name: 'DOS', - }, - { - name: 'TRES', - }, - ], - ctype: 'miscellaneous', - subtype: 'enum', - description: '', - file: 'src/stories/component-with-enums/enums.component.ts', - }, - { - name: 'EnumStringValues', - childs: [ - { - name: 'PRIMARY', - value: 'PRIMARY', - }, - { - name: 'SECONDARY', - value: 'SECONDARY', - }, - { - name: 'TERTIARY', - value: 'TERTIARY', - }, - ], - ctype: 'miscellaneous', - subtype: 'enum', - description: '', - file: 'src/stories/component-with-enums/enums.component.ts', - }, - ], - }, - } as CompodocJson; +const extractedNames = async () => { + const { extractArgTypesFromData } = await import('./compodoc.ts'); + return Object.keys(extractArgTypesFromData(componentData as never)); }; -describe('extractType', () => { - describe('with compodoc type', () => { - setCompodocJson(getDummyCompodocJson()); - it.each([ - ['string', { name: 'string' }], - ['boolean', { name: 'boolean' }], - ['number', { name: 'number' }], - // ['object', { name: 'object' }], // seems to be wrong | TODO: REVISIT - // ['foo', { name: 'other', value: 'empty-enum' }], // seems to be wrong | TODO: REVISIT - [null, { name: 'other', value: 'void' }], - [undefined, { name: 'other', value: 'void' }], - // ['T[]', { name: 'other', value: 'empty-enum' }], // seems to be wrong | TODO: REVISIT - ['[]', { name: 'other', value: 'empty-enum' }], - ['"primary" | "secondary"', { name: 'enum', value: ['primary', 'secondary'] }], - ['TypeAlias', { name: 'enum', value: ['Type Alias 1', 'Type Alias 2', 'Type Alias 3'] }], - // ['EnumNumeric', { name: 'other', value: 'empty-enum' }], // seems to be wrong | TODO: REVISIT - // ['EnumNumericInitial', { name: 'other', value: 'empty-enum' }], // seems to be wrong | TODO: REVISIT - ['EnumStringValues', { name: 'enum', value: ['PRIMARY', 'SECONDARY', 'TERTIARY'] }], - ])('%s', (compodocType, expected) => { - expect(extractType(makeProperty(compodocType), null)).toEqual(expected); - }); +beforeEach(() => { + vi.resetModules(); + vi.stubGlobal('FEATURES', { angularFilterNonInputControls: false }); + vi.stubGlobal('__STORYBOOK_COMPODOC_JSON__', compodocJson); +}); + +afterEach(() => { + vi.unstubAllGlobals(); +}); + +describe('resolving propsTable for the Compodoc pipeline', () => { + it("maps 'inputs' onto the legacy inputs-only filter", async () => { + vi.stubGlobal('STORYBOOK_ANGULAR_OPTIONS', { zoneless: true, propsTable: 'inputs' }); + + await expect(extractedNames()).resolves.toEqual(['label']); + }); + + it("reads 'api' as all, because Compodoc's visibility is not interpretable here", async () => { + vi.stubGlobal('STORYBOOK_ANGULAR_OPTIONS', { zoneless: true, propsTable: 'api' }); + + await expect(extractedNames()).resolves.toEqual(['note', 'label']); }); - describe('without compodoc type', () => { - it.each([ - ['string', { name: 'string' }], - ['', { name: 'string' }], - [false, { name: 'boolean' }], - [10, { name: 'number' }], - // [['abc'], { name: 'object' }], // seems to be wrong | TODO: REVISIT - // [{ foo: 1 }, { name: 'other', value: 'empty-enum' }], // seems to be wrong | TODO: REVISIT - [undefined, { name: 'other', value: 'void' }], - ])('%s', (defaultValue, expected) => { - expect(extractType(makeProperty(null), defaultValue)).toEqual(expected); - }); + it("overrides the deprecated feature whenever a mode is defined, 'all' included", async () => { + vi.stubGlobal('FEATURES', { angularFilterNonInputControls: true }); + vi.stubGlobal('STORYBOOK_ANGULAR_OPTIONS', { zoneless: true, propsTable: 'all' }); + + await expect(extractedNames()).resolves.toEqual(['note', 'label']); + }); + + it('falls back to the deprecated feature when the define never ran', async () => { + vi.stubGlobal('FEATURES', { angularFilterNonInputControls: true }); + + await expect(extractedNames()).resolves.toEqual(['label']); }); }); diff --git a/code/frameworks/angular-vite/src/client/compodoc.ts b/code/frameworks/angular-vite/src/client/compodoc.ts index 34bcd15e1248..0e87b3c5f4db 100644 --- a/code/frameworks/angular-vite/src/client/compodoc.ts +++ b/code/frameworks/angular-vite/src/client/compodoc.ts @@ -1,13 +1,37 @@ /** * The Compodoc parsing and its browser adapter live in `@storybook/angular-compodoc`, shared with - * `@storybook/angular` and with this package's Node docgen worker. This module only keeps the - * historical `client/compodoc` import path alive. + * `@storybook/angular` and with this package's Node docgen worker. This module keeps the + * historical `client/compodoc` import path alive and resolves this framework's `propsTable` + * option for the shared adapter. */ +import { + extractArgTypes as extractArgTypesShared, + extractArgTypesFromData as extractArgTypesFromDataShared, +} from '@storybook/angular-compodoc/browser'; + +/** + * Compodoc encodes member visibility only as raw TypeScript `SyntaxKind` numbers, which the frozen + * legacy pipeline deliberately does not interpret, so `api` is not answerable here and reads as + * `all`; the preset warns when a user asks for it. Left undefined when Vite's `define` never ran, + * which is how a portable-stories host imports this file, so the shared adapter falls back to the + * deprecated `angularFilterNonInputControls` feature. + */ +const filterNonInputControls = + typeof STORYBOOK_ANGULAR_OPTIONS === 'undefined' || + STORYBOOK_ANGULAR_OPTIONS.propsTable === undefined + ? undefined + : STORYBOOK_ANGULAR_OPTIONS.propsTable === 'inputs'; + +export const extractArgTypes = (component: Parameters[0]) => + extractArgTypesShared(component, { filterNonInputControls }); + +export const extractArgTypesFromData = ( + componentData: Parameters[0] +) => extractArgTypesFromDataShared(componentData, { filterNonInputControls }); + export { checkValidCompodocJson, checkValidComponentOrDirective, - extractArgTypes, - extractArgTypesFromData, extractComponentDescription, extractType, findComponentByName, diff --git a/code/frameworks/angular-vite/src/client/config.ts b/code/frameworks/angular-vite/src/client/config.ts index 766616d3391a..202ded20ce0b 100644 --- a/code/frameworks/angular-vite/src/client/config.ts +++ b/code/frameworks/angular-vite/src/client/config.ts @@ -6,13 +6,17 @@ export { decorateStory as applyDecorators } from './decorateStory.ts'; import { enhanceArgTypes } from 'storybook/internal/docs-tools'; import type { ArgTypesEnhancer, Parameters } from 'storybook/internal/types'; +import { global } from '@storybook/global'; + +import type { Component, Directive } from './compodoc-types.ts'; import { extractArgTypes, extractComponentDescription } from './compodoc.ts'; export const parameters: Parameters = { renderer: 'angular', docs: { story: { inline: true }, - extractArgTypes, + extractArgTypes: (component: Component | Directive) => + global.FEATURES?.experimentalDocgenServer === true ? {} : extractArgTypes(component), extractComponentDescription, }, }; diff --git a/code/frameworks/angular-vite/src/client/renderer/AbstractRenderer.ts b/code/frameworks/angular-vite/src/client/renderer/AbstractRenderer.ts index ec2af3cf9dc7..bb9b496b8028 100644 --- a/code/frameworks/angular-vite/src/client/renderer/AbstractRenderer.ts +++ b/code/frameworks/angular-vite/src/client/renderer/AbstractRenderer.ts @@ -19,6 +19,7 @@ type StoryRenderInfo = { declare global { const STORYBOOK_ANGULAR_OPTIONS: { zoneless: boolean; + propsTable?: import('@storybook/angular-cm').PropsTableMode; }; } diff --git a/code/frameworks/angular-vite/src/docgen/build-docgen.integration.test.ts b/code/frameworks/angular-vite/src/docgen/build-docgen.integration.test.ts index 8f473e9ec5a6..a044b0b833d7 100644 --- a/code/frameworks/angular-vite/src/docgen/build-docgen.integration.test.ts +++ b/code/frameworks/angular-vite/src/docgen/build-docgen.integration.test.ts @@ -31,7 +31,7 @@ it('builds a real payload through the TypeScript-backed analyzer', async () => { { entry }, { manager, - options: {}, + options: { propsTable: 'api' }, logger: { warn: vi.fn(), debug: vi.fn() }, resolvePath: () => STORY_PATH, } diff --git a/code/frameworks/angular-vite/src/docgen/build-docgen.test.ts b/code/frameworks/angular-vite/src/docgen/build-docgen.test.ts index 6cca11e98adf..0cdf591abbcc 100644 --- a/code/frameworks/angular-vite/src/docgen/build-docgen.test.ts +++ b/code/frameworks/angular-vite/src/docgen/build-docgen.test.ts @@ -76,7 +76,7 @@ const managerReturning = (meta: AngularComponentMetaResult | undefined) => ({ const context = ( manager: AngularComponentMetaSource, - options: BuildDocgenContext['options'] = {} + options: BuildDocgenContext['options'] = { propsTable: 'all' } ): BuildDocgenContext => ({ manager, options, logger }); describe('buildDocgenPayload', () => { @@ -227,25 +227,23 @@ describe('buildDocgenPayload', () => { }); }); - it('honours `angularFilterNonInputControls`', () => { + it('hands `propsTable` to the conversion', () => { givenStoryFile(); const classMeta = componentEntry({ - propertiesClass: [{ name: 'internal', type: 'string', optional: false }], + propertiesClass: [ + { name: 'note', type: 'string', optional: false }, + { name: 'cdr', type: 'ChangeDetectorRef', optional: false, visibility: 'private' }, + ], }); - - expect( - Object.keys( - buildDocgenPayload({ entry }, context(managerReturning(metaFor(classMeta))))?.argTypes ?? {} - ) - ).toEqual(['internal', 'label']); - expect( + const argNames = (options: BuildDocgenContext['options']) => Object.keys( - buildDocgenPayload( - { entry }, - context(managerReturning(metaFor(classMeta)), { angularFilterNonInputControls: true }) - )?.argTypes ?? {} - ) - ).toEqual(['label']); + buildDocgenPayload({ entry }, context(managerReturning(metaFor(classMeta)), options)) + ?.argTypes ?? {} + ); + + expect(argNames({ propsTable: 'all' })).toEqual(['note', 'cdr', 'label']); + expect(argNames({ propsTable: 'api' })).toEqual(['note', 'label']); + expect(argNames({ propsTable: 'inputs' })).toEqual(['label']); }); describe('component resolution', () => { diff --git a/code/frameworks/angular-vite/src/docgen/build-docgen.ts b/code/frameworks/angular-vite/src/docgen/build-docgen.ts index f4ac230749b6..10d799d392b0 100644 --- a/code/frameworks/angular-vite/src/docgen/build-docgen.ts +++ b/code/frameworks/angular-vite/src/docgen/build-docgen.ts @@ -12,13 +12,14 @@ import type { AngularClassMeta, AngularComponentMetaResult, ParsingLogger, + PropsTableMode, } from '@storybook/angular-cm'; import { extractArgTypesFromData } from '@storybook/angular-cm'; import { resolveStoryComponent } from './resolve-component.ts'; // Structured-cloned onto the worker thread, so every field must be plain JSON data. export interface AngularDocgenOptions { - angularFilterNonInputControls?: boolean; + propsTable: PropsTableMode; } export interface SnippetEnum { @@ -39,6 +40,7 @@ export interface AngularComponentSnippetMeta { } export type AngularDocgenPayload = DocgenPayload & { + // The analyzer's record for the class, never filtered by `propsTable`. angularComponentMeta?: AngularComponentSnippetMeta; }; @@ -182,7 +184,7 @@ export const buildDocgenPayload = ( const argTypes = extractArgTypesFromData(meta.entry, { metadataJson: meta.json, - filterNonInputControls: options.angularFilterNonInputControls, + propsTable: options.propsTable, logger, }) as StrictArgTypes; diff --git a/code/frameworks/angular-vite/src/docgen/docgen-worker.test.ts b/code/frameworks/angular-vite/src/docgen/docgen-worker.test.ts index 55a7af6bf78d..dc08589a3831 100644 --- a/code/frameworks/angular-vite/src/docgen/docgen-worker.test.ts +++ b/code/frameworks/angular-vite/src/docgen/docgen-worker.test.ts @@ -90,7 +90,9 @@ describe('createDocgenProvider', () => { const next = vi.fn(passthrough); await expect( - createDocgenProvider()(next)({ entry: { ...entry, importPath: './button.component.ts' } }) + createDocgenProvider({ propsTable: 'api' })(next)({ + entry: { ...entry, importPath: './button.component.ts' }, + }) ).resolves.toBeUndefined(); expect(next).toHaveBeenCalledOnce(); @@ -101,7 +103,7 @@ describe('createDocgenProvider', () => { it('merges over downstream output on success', async () => { vi.mocked(buildDocgenPayload).mockReturnValue(ours); - const payload = await createDocgenProvider()(async () => ({ + const payload = await createDocgenProvider({ propsTable: 'api' })(async () => ({ ...downstream, somethingElse: 'kept', }))({ entry }); @@ -115,21 +117,27 @@ describe('createDocgenProvider', () => { vi.mocked(buildDocgenPayload).mockReturnValue(errored); const next = vi.fn(async () => downstream); - await expect(createDocgenProvider()(next)({ entry })).resolves.toEqual(downstream); + await expect(createDocgenProvider({ propsTable: 'api' })(next)({ entry })).resolves.toEqual( + downstream + ); expect(next).toHaveBeenCalledOnce(); }); it('reports its own error only when no other provider described the component', async () => { vi.mocked(buildDocgenPayload).mockReturnValue(errored); - await expect(createDocgenProvider()(passthrough)({ entry })).resolves.toEqual(errored); + await expect( + createDocgenProvider({ propsTable: 'api' })(passthrough)({ entry }) + ).resolves.toEqual(errored); }); it('delegates downstream when it has no payload for the component', async () => { vi.mocked(buildDocgenPayload).mockReturnValue(undefined); const next = vi.fn(async () => downstream); - await expect(createDocgenProvider()(next)({ entry })).resolves.toEqual(downstream); + await expect(createDocgenProvider({ propsTable: 'api' })(next)({ entry })).resolves.toEqual( + downstream + ); expect(next).toHaveBeenCalledOnce(); }); @@ -139,12 +147,14 @@ describe('createDocgenProvider', () => { throw failure; }); - await expect(createDocgenProvider()(passthrough)({ entry })).rejects.toBe(failure); + await expect(createDocgenProvider({ propsTable: 'api' })(passthrough)({ entry })).rejects.toBe( + failure + ); }); it('owns one watching analyzer for its lifetime and recycles after each extraction', async () => { vi.mocked(buildDocgenPayload).mockReturnValue(ours); - const provider = createDocgenProvider()(passthrough); + const provider = createDocgenProvider({ propsTable: 'api' })(passthrough); await provider({ entry }); await provider({ entry }); @@ -159,15 +169,15 @@ describe('createDocgenProvider', () => { it('threads a structured-cloneable options bag and the manager into the payload builder', async () => { vi.mocked(buildDocgenPayload).mockReturnValue(ours); - await createDocgenProvider(structuredClone({ angularFilterNonInputControls: true }))( - passthrough - )({ entry }); + await createDocgenProvider(structuredClone({ propsTable: 'inputs' } as const))(passthrough)({ + entry, + }); expect(buildDocgenPayload).toHaveBeenCalledExactlyOnceWith( { entry }, { manager: analyzer.instances[0], - options: { angularFilterNonInputControls: true }, + options: { propsTable: 'inputs' }, logger: expect.objectContaining({ warn: expect.any(Function), debug: expect.any(Function), @@ -180,7 +190,7 @@ describe('createDocgenProvider', () => { analyzer.failConstruction = true; vi.mocked(logger.warn).mockImplementation(() => {}); const next = vi.fn(async () => downstream); - const provider = createDocgenProvider()(next); + const provider = createDocgenProvider({ propsTable: 'api' })(next); await expect(provider({ entry })).resolves.toEqual(downstream); await expect(provider({ entry })).resolves.toEqual(downstream); diff --git a/code/frameworks/angular-vite/src/docgen/docgen-worker.ts b/code/frameworks/angular-vite/src/docgen/docgen-worker.ts index c898749ec50f..bc85eb89edde 100644 --- a/code/frameworks/angular-vite/src/docgen/docgen-worker.ts +++ b/code/frameworks/angular-vite/src/docgen/docgen-worker.ts @@ -34,7 +34,7 @@ const createManager = async (): Promise * Build the Angular docgen middleware, holding one analyzer for the worker's lifetime so its * language services stay warm across components. */ -export const createDocgenProvider = (options: AngularDocgenOptions = {}): DocgenMiddleware => { +export const createDocgenProvider = (options: AngularDocgenOptions): DocgenMiddleware => { let managerPromise: Promise | undefined; return (nextDocgen: DocgenProvider): DocgenProvider => diff --git a/code/frameworks/angular-vite/src/docgen/preset.test.ts b/code/frameworks/angular-vite/src/docgen/preset.test.ts index cebf568a689b..458369847f77 100644 --- a/code/frameworks/angular-vite/src/docgen/preset.test.ts +++ b/code/frameworks/angular-vite/src/docgen/preset.test.ts @@ -18,6 +18,9 @@ const optionsWith = ( if (key === 'framework') { return { name: '@storybook/angular-vite', options: frameworkOptions }; } + if (key === 'frameworkOptions') { + return frameworkOptions; + } return undefined; }, }, @@ -42,11 +45,32 @@ describe('experimental_docgenProvider', () => { it('contributes the worker descriptor when the flag is on and Compodoc is not opted out', async () => { const result = await experimental_docgenProvider( [], - optionsWith({ experimentalDocgenServer: true, angularFilterNonInputControls: true }) + optionsWith({ experimentalDocgenServer: true }) ); expect(result).toHaveLength(1); expect(result[0].moduleSpecifier).toContain('docgen-worker'); - expect(result[0].options).toEqual({ angularFilterNonInputControls: true }); + expect(result[0].options).toEqual({ propsTable: 'api' }); + }); + + it('hands the worker the mode the deprecated feature maps onto', async () => { + const result = await experimental_docgenProvider( + [], + optionsWith({ experimentalDocgenServer: true, angularFilterNonInputControls: true }) + ); + + expect(result[0].options).toEqual({ propsTable: 'inputs' }); + }); + + it('hands the worker the framework option, which outranks the deprecated feature', async () => { + const result = await experimental_docgenProvider( + [], + optionsWith( + { experimentalDocgenServer: true, angularFilterNonInputControls: true }, + { propsTable: 'all' } + ) + ); + + expect(result[0].options).toEqual({ propsTable: 'all' }); }); }); diff --git a/code/frameworks/angular-vite/src/docgen/preset.ts b/code/frameworks/angular-vite/src/docgen/preset.ts index 922ec9b869a4..7e7b97f1ff63 100644 --- a/code/frameworks/angular-vite/src/docgen/preset.ts +++ b/code/frameworks/angular-vite/src/docgen/preset.ts @@ -10,6 +10,7 @@ import type { import { fileURLToPath } from 'node:url'; import { resolveCompodocConfig } from '../compodoc-config.ts'; +import { resolvePropsTable } from '../props-table.ts'; import type { AngularDocgenOptions } from './build-docgen.ts'; /** Contribute the descriptor for the worker module core imports and runs off the main thread. */ @@ -33,7 +34,7 @@ export const experimental_docgenProvider = async ( import.meta.resolve('@storybook/angular-vite/internal/docgen-worker') ), options: { - angularFilterNonInputControls: features?.angularFilterNonInputControls, + propsTable: resolvePropsTable(await options?.presets?.apply('frameworkOptions'), features), }, }; diff --git a/code/frameworks/angular-vite/src/preset.test.ts b/code/frameworks/angular-vite/src/preset.test.ts index 28f92c7e44ae..956f1dde1fce 100644 --- a/code/frameworks/angular-vite/src/preset.test.ts +++ b/code/frameworks/angular-vite/src/preset.test.ts @@ -1,5 +1,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { logger } from 'storybook/internal/node-logger'; + import { resolve } from 'node:path'; import { mergeConfig, normalizePath } from 'vite'; @@ -128,3 +130,52 @@ describe('viteFinal Compodoc generation', () => { expect(ensureCompodocDocumentation).not.toHaveBeenCalled(); }); }); + +describe('viteFinal props-table wiring', () => { + const optionsWith = ( + frameworkOptions: Record, + featureFlags: Record = {} + ) => + ({ + configDir: resolve(WORKSPACE_ROOT, '.storybook'), + angularBuilderContext: { workspaceRoot: WORKSPACE_ROOT }, + presets: { + apply: async (key: string, fallback?: unknown) => { + if (key === 'framework') { + return { options: frameworkOptions }; + } + return key === 'features' ? featureFlags : fallback; + }, + }, + }) as unknown as StandaloneOptions; + + const definedMode = async ( + frameworkOptions: Record, + featureFlags: Record = {} + ) => { + const result = (await viteFinal( + { root: WORKSPACE_ROOT }, + optionsWith(frameworkOptions, featureFlags) + )) as any; + return JSON.parse(result.define.STORYBOOK_ANGULAR_OPTIONS).propsTable; + }; + + it('hands the preview the resolved mode, which is how the flag-off path reads it', async () => { + await expect(definedMode({})).resolves.toBe('api'); + await expect(definedMode({ propsTable: 'all' })).resolves.toBe('all'); + await expect(definedMode({}, { angularFilterNonInputControls: true })).resolves.toBe('inputs'); + }); + + it('warns from here, because the docgen preset never runs with the feature off', async () => { + const warn = vi.spyOn(logger, 'warn').mockImplementation(() => {}); + try { + await viteFinal({ root: WORKSPACE_ROOT }, optionsWith({ propsTable: 'api' })); + + expect(warn.mock.calls.map(([message]) => String(message)).join('\n')).toContain( + 'experimentalDocgenServer' + ); + } finally { + warn.mockRestore(); + } + }); +}); diff --git a/code/frameworks/angular-vite/src/preset.ts b/code/frameworks/angular-vite/src/preset.ts index 8c993b3b7380..9ea3deabf406 100644 --- a/code/frameworks/angular-vite/src/preset.ts +++ b/code/frameworks/angular-vite/src/preset.ts @@ -13,6 +13,7 @@ import { resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; import { resolveCompodocConfig } from './compodoc-config.ts'; +import { resolvePropsTable, warnAboutPropsTable } from './props-table.ts'; import { ensureCompodocDocumentation } from './compodoc/ensure-documentation.ts'; import type { StandaloneOptions } from './builders/utils/standalone-options.ts'; import type { UserConfig, Plugin } from 'vite'; @@ -110,6 +111,11 @@ export const viteFinal = async (config: UserConfig, options?: StandaloneOptions) }); } + // @ts-expect-error same as `framework` above: `options` is optional in the signature only + const features = await options.presets.apply('features', {}); + const propsTable = resolvePropsTable(framework.options, features); + warnAboutPropsTable(framework.options, features); + const zoneless = resolveZoneless(options?.angularBuilderOptions); const angularPlugins = angular({ jit: typeof framework.options?.jit !== 'undefined' ? framework.options?.jit : true, @@ -195,6 +201,7 @@ export const viteFinal = async (config: UserConfig, options?: StandaloneOptions) define: { STORYBOOK_ANGULAR_OPTIONS: JSON.stringify({ zoneless: !!zoneless, + propsTable, }), }, }); diff --git a/code/frameworks/angular-vite/src/props-table.test.ts b/code/frameworks/angular-vite/src/props-table.test.ts new file mode 100644 index 000000000000..f2aacdffdb03 --- /dev/null +++ b/code/frameworks/angular-vite/src/props-table.test.ts @@ -0,0 +1,100 @@ +import { describe, expect, it, vi } from 'vitest'; + +import { deprecate, logger } from 'storybook/internal/node-logger'; + +import { resolvePropsTable, warnAboutPropsTable } from './props-table.ts'; + +// The shared setup's node-logger mock keeps the real `deprecate`, which logs past the mocked +// `logger`, so deprecations are only observable through a file-local mock. +vi.mock('storybook/internal/node-logger', () => ({ + logger: { warn: vi.fn() }, + deprecate: vi.fn(), +})); + +const warnings = ( + frameworkOptions: Record, + features: Record = {} +) => { + vi.mocked(logger.warn).mockClear(); + vi.mocked(deprecate).mockClear(); + warnAboutPropsTable(frameworkOptions, features); + return [...vi.mocked(logger.warn).mock.calls, ...vi.mocked(deprecate).mock.calls].map( + ([message]) => String(message) + ); +}; + +describe('resolvePropsTable', () => { + it('defaults to api', () => { + expect(resolvePropsTable({}, {})).toBe('api'); + }); + + it('reads the framework option', () => { + expect(resolvePropsTable({ propsTable: 'all' }, {})).toBe('all'); + }); + + it('maps the deprecated flag onto the ladder', () => { + expect(resolvePropsTable({}, { angularFilterNonInputControls: true })).toBe('inputs'); + expect(resolvePropsTable({}, { angularFilterNonInputControls: false })).toBe('all'); + }); + + it('lets an explicit propsTable win over the deprecated flag', () => { + expect(resolvePropsTable({ propsTable: 'api' }, { angularFilterNonInputControls: true })).toBe( + 'api' + ); + }); + + it('defaults when core reports no framework options at all', () => { + expect(resolvePropsTable(null, {})).toBe('api'); + }); + + it('falls back past a value that is not a mode', () => { + expect(resolvePropsTable({ propsTable: 'API' as never }, {})).toBe('api'); + expect( + resolvePropsTable({ propsTable: 'input' as never }, { angularFilterNonInputControls: true }) + ).toBe('inputs'); + }); +}); + +describe('warnAboutPropsTable', () => { + it('names propsTable as the replacement for the deprecated flag', () => { + const messages = warnings({}, { angularFilterNonInputControls: true }); + + expect(messages).toHaveLength(1); + expect(messages[0]).toContain('angularFilterNonInputControls'); + expect(messages[0]).toContain("propsTable: 'inputs'"); + }); + + it('says the flag is ignored when propsTable is set too', () => { + const messages = warnings( + { propsTable: 'all' }, + { angularFilterNonInputControls: true, experimentalDocgenServer: true } + ); + + expect(messages).toHaveLength(1); + expect(messages[0]).toContain('takes precedence'); + }); + + it('stays quiet when neither the flag nor an unsupported mode is configured', () => { + expect(warnings({}, { experimentalDocgenServer: true })).toEqual([]); + expect(warnings({ propsTable: 'all' })).toEqual([]); + }); + + it('warns that an explicit api needs the docgen server, without downgrading it', () => { + const messages = warnings({ propsTable: 'api' }); + + expect(messages).toHaveLength(1); + expect(messages[0]).toContain('experimentalDocgenServer'); + }); + + it('does not warn about the api default, which nobody asked for', () => { + expect(warnings({})).toEqual([]); + }); + + it('calls out a value that is not a mode instead of half-applying it', () => { + const messages = warnings({ propsTable: 'input' }); + + expect(messages).toHaveLength(1); + expect(messages[0]).toContain('"input"'); + expect(messages[0]).toContain("'inputs'"); + }); +}); diff --git a/code/frameworks/angular-vite/src/props-table.ts b/code/frameworks/angular-vite/src/props-table.ts new file mode 100644 index 000000000000..f717d57a694f --- /dev/null +++ b/code/frameworks/angular-vite/src/props-table.ts @@ -0,0 +1,72 @@ +import { deprecate, logger } from 'storybook/internal/node-logger'; +import type { StorybookFeatures } from 'storybook/internal/types'; + +import type { PropsTableMode } from '@storybook/angular-cm'; +import type { FrameworkOptions } from './types.ts'; + +type PropsTableInput = Pick | null | undefined; +type Features = + | Pick + | undefined; + +const MODES: readonly PropsTableMode[] = ['all', 'api', 'inputs']; + +// A misspelt mode arrives through untyped JS configs; carrying it onward would half-apply it, with +// each pipeline reading the junk string differently. +const configuredMode = (frameworkOptions: PropsTableInput): PropsTableMode | undefined => { + const configured = frameworkOptions?.propsTable; + return MODES.includes(configured as PropsTableMode) ? configured : undefined; +}; + +/** + * Resolves the one switch that decides which members the props table renders. + * + * `angularFilterNonInputControls` is the deprecated spelling of the two outer rungs of the same + * ladder, so it maps onto a mode rather than surviving as a second switch that could disagree. + */ +export const resolvePropsTable = ( + frameworkOptions: PropsTableInput, + features: Features +): PropsTableMode => { + const deprecatedFlag = features?.angularFilterNonInputControls; + const inherited = deprecatedFlag === undefined ? undefined : deprecatedFlag ? 'inputs' : 'all'; + + return configuredMode(frameworkOptions) ?? inherited ?? 'api'; +}; + +/** + * Reports every props-table setting that will not do what it says. + * + * Call this from a hook that runs whatever the feature flags say: the docgen preset is skipped + * entirely when `experimentalDocgenServer` is off, which is exactly the case one of these warnings + * is about. + */ +export const warnAboutPropsTable = ( + frameworkOptions: PropsTableInput, + features: Features +): void => { + const raw = frameworkOptions?.propsTable; + const configured = configuredMode(frameworkOptions); + if (raw !== undefined && configured === undefined) { + logger.warn( + `Ignoring the unknown \`propsTable\` value ${JSON.stringify(raw)}; expected 'all', 'api' or 'inputs'.` + ); + } + + if (features?.angularFilterNonInputControls !== undefined) { + const mode = resolvePropsTable(frameworkOptions, features); + deprecate( + `The \`angularFilterNonInputControls\` feature is deprecated and will be removed in Storybook 11. ` + + (configured !== undefined + ? `The \`propsTable: '${mode}'\` framework option takes precedence over it, so the feature has no effect and can be removed.` + : `Replace it with the \`propsTable: '${mode}'\` option on your \`@storybook/angular-vite\` framework.`) + ); + } + + if (configured === 'api' && features?.experimentalDocgenServer !== true) { + logger.warn( + `\`propsTable: 'api'\` needs the \`experimentalDocgenServer\` feature, which is off, so the props table keeps showing every member. ` + + `Enable it with \`features: { experimentalDocgenServer: true }\`, or set \`propsTable: 'all'\` to say you want every member.` + ); + } +}; diff --git a/code/frameworks/angular-vite/src/types.ts b/code/frameworks/angular-vite/src/types.ts index ea270d093f69..489f3af8aced 100644 --- a/code/frameworks/angular-vite/src/types.ts +++ b/code/frameworks/angular-vite/src/types.ts @@ -2,6 +2,7 @@ import type { CompatibleString } from 'storybook/internal/types'; import type { StorybookConfig as StorybookConfigBase } from 'storybook/internal/types'; +import type { PropsTableMode } from '@storybook/angular-cm'; import type { BuilderOptions, StorybookConfigVite } from '@storybook/builder-vite'; type FrameworkName = CompatibleString<'@storybook/angular-vite'>; @@ -15,6 +16,23 @@ export type FrameworkOptions = { tsconfig?: string; compodoc?: boolean; compodocArgs?: string[]; + /** + * Which members the props table renders, as a ladder: + * + * - `all`: every member. + * - `api`: the component's template-facing surface, meaning every declared input and output + * whatever its TypeScript visibility (Angular only honours access modifiers on bindings behind + * the opt-in `strictInputAccessModifiers`), plus the properties and methods that are not + * `private`, ES `#`, or carrying a JSDoc `internal` tag (a declared non-API). + * - `inputs`: the inputs section alone. + * + * Tag a member `@ignore` to drop it whatever this says. + * + * `api` needs `features.experimentalDocgenServer`; without it only `all` and `inputs` apply. + * + * @default 'api' + */ + propsTable?: PropsTableMode; }; type StorybookConfigFramework = { diff --git a/code/lib/angular-cm/src/analyzer/members.test.ts b/code/lib/angular-cm/src/analyzer/members.test.ts index 3f0e4cb24d34..4d65d36d6763 100644 --- a/code/lib/angular-cm/src/analyzer/members.test.ts +++ b/code/lib/angular-cm/src/analyzer/members.test.ts @@ -27,8 +27,8 @@ afterEach(() => { vi.restoreAllMocks(); }); -// Mirrors what the docgen worker passes for analyzer-produced records. -const ANALYZER_EXTRACT_OPTIONS = { filterNonInputControls: undefined } as const; +// These tests are about what the analyzer records, so nothing here may be filtered on the way out. +const ANALYZER_EXTRACT_OPTIONS = { propsTable: 'all' } as const; const soleComponent = (meta: ReturnType) => meta.components[0] as Directive; @@ -1109,12 +1109,15 @@ describe('real-world JSDoc, visibility and accessor edge cases', () => { }); }); - it('drops an undecorated private or protected accessor pair, but keeps a public one', () => { - const properties = names(componentIn(SOURCE).propertiesClass); + it('records an undecorated accessor pair with its visibility, whatever that visibility is', () => { + const properties = componentIn(SOURCE).propertiesClass; - expect(properties).toContain('publicPair'); - expect(properties).not.toContain('internalState'); - expect(properties).not.toContain('errorPresent'); + expect(names(properties)).toEqual( + expect.arrayContaining(['publicPair', 'internalState', 'errorPresent']) + ); + expect(byName(properties, 'publicPair').visibility).toBeUndefined(); + expect(byName(properties, 'internalState').visibility).toBe('private'); + expect(byName(properties, 'errorPresent').visibility).toBe('protected'); }); it('treats @Output() on a getter as an output, alias honoured', () => { @@ -1185,12 +1188,13 @@ describe('member identity is the declared field, not the emitted name', () => { expect(byName(properties, 'asGetter').decorators).toEqual([{ name: 'ContentChild' }]); }); - it('emits publicly visible parameter properties, bare `readonly` included, with their tags', () => { + it('emits parameter properties, bare `readonly` included, with their tags and visibility', () => { const component = componentIn(SOURCE); expect(names(component.propertiesClass)).toContain('pageSize'); expect(names(component.propertiesClass)).toContain('pageIndex'); - expect(names(component.propertiesClass)).not.toContain('hidden'); + expect(byName(component.propertiesClass, 'hidden').visibility).toBe('private'); + expect(byName(component.propertiesClass, 'pageSize').visibility).toBeUndefined(); expect(byName(component.propertiesClass, 'legacySize').jsdoctags).toMatchObject([ { tagName: { text: 'deprecated' } }, ]); @@ -1199,10 +1203,18 @@ describe('member identity is the declared field, not the emitted name', () => { it('says why a member it left out is missing', () => { const debug = vi.spyOn(logger, 'debug').mockImplementation(() => {}); - analyze(SOURCE); + analyze(` + import { Component } from '@angular/core'; + + @Component({ selector: 'sb-ignored', template: '' }) + export class IgnoredComponent { + /** @ignore */ + hidden = 1; + } + `); expect(debug.mock.calls.map(([message]) => message)).toContain( - '[angular-cm] MemberIdentityComponent.hidden left out of docgen: a private or protected parameter property' + '[angular-cm] IgnoredComponent.hidden left out of docgen: tagged @ignore' ); }); @@ -1217,3 +1229,179 @@ describe('member identity is the declared field, not the emitted name', () => { ).toEqual(['string', 'number']); }); }); + +describe('visibility and @internal are recorded, not acted on', () => { + const SOURCE = ` + import { ChangeDetectorRef, Component, Input, inject } from '@angular/core'; + + @Component({ selector: 'sb-visibility', template: '' }) + export class VisibilityComponent { + @Input() title = ''; + + private readonly cdr = inject(ChangeDetectorRef); + + protected helperLabel = 'help'; + + publicNote = 'note'; + + /** @internal */ + buildId = 'build-1'; + + private stash(): void {} + + protected assist(): void {} + + /** @internal */ + reset(): void {} + } + `; + + it('marks a private property and leaves a public one unmarked', () => { + const component = componentIn(SOURCE); + + expect(byName(component.propertiesClass, 'cdr').visibility).toBe('private'); + expect(byName(component.propertiesClass, 'publicNote').visibility).toBeUndefined(); + expect(byName(component.inputsClass, 'title').visibility).toBeUndefined(); + }); + + it('marks a protected property, which stays bindable from a template', () => { + expect(byName(componentIn(SOURCE).propertiesClass, 'helperLabel').visibility).toBe('protected'); + }); + + it('marks private and protected methods', () => { + const component = componentIn(SOURCE); + + expect(byName(component.methodsClass, 'stash').visibility).toBe('private'); + expect(byName(component.methodsClass, 'assist').visibility).toBe('protected'); + }); + + it('marks an @internal property and method regardless of their visibility', () => { + const component = componentIn(SOURCE); + + expect(byName(component.propertiesClass, 'buildId').internal).toBe(true); + expect(byName(component.propertiesClass, 'buildId').visibility).toBeUndefined(); + expect(byName(component.methodsClass, 'reset').internal).toBe(true); + }); + + it('keeps every one of them in the payload for the extractor to decide on', () => { + const component = componentIn(SOURCE); + + expect(names(component.propertiesClass)).toEqual( + expect.arrayContaining(['cdr', 'helperLabel', 'publicNote', 'buildId']) + ); + expect(names(component.methodsClass)).toEqual( + expect.arrayContaining(['stash', 'assist', 'reset']) + ); + }); +}); + +describe('what `propsTable` does with the recorded visibility', () => { + const SOURCE = ` + import { Component, EventEmitter, Input, Output, model } from '@angular/core'; + + @Component({ selector: 'sb-props-table', template: '' }) + export class PropsTableComponent { + constructor(private readonly cdr: string, protected readonly host: string) {} + + @Input() private density = 'compact'; + + @Output() private densityChange = new EventEmitter(); + + /** @internal */ + @Input() experimentalKnob = ''; + + /** @internal */ + @Output() experimentalChanged = new EventEmitter(); + + protected helperLabel = 'help'; + + private pageCount = 10; + + #secret = 'hidden'; + + /** @internal */ + buildId = 'build-1'; + + /** @internal */ + draft = model(''); + + published = model(''); + } + `; + + const argNames = (propsTable: 'all' | 'api' | 'inputs') => + Object.keys( + extractArgTypesFromData(componentIn(SOURCE), { metadataJson: undefined, propsTable }) + ); + + it('documents every recorded member under `all`', () => { + expect(argNames('all')).toEqual( + expect.arrayContaining([ + 'cdr', + 'host', + 'density', + 'densityChange', + 'helperLabel', + 'pageCount', + '#secret', + 'buildId', + 'draft', + 'draftChange', + ]) + ); + }); + + it('keeps a private input and output under `api`, which a parent template can still bind', () => { + expect(argNames('api')).toEqual(expect.arrayContaining(['density', 'densityChange'])); + }); + + it("keeps `protected` under `api`, which the component's own template reads", () => { + expect(argNames('api')).toEqual(expect.arrayContaining(['helperLabel', 'host'])); + }); + + it('drops what nothing can reach under `api`', () => { + expect(argNames('api')).not.toContain('cdr'); + expect(argNames('api')).not.toContain('pageCount'); + expect(argNames('api')).not.toContain('#secret'); + }); + + it('drops an @internal input and output in api and inputs, keeping them in all', () => { + expect(argNames('all')).toEqual( + expect.arrayContaining(['experimentalKnob', 'experimentalChanged']) + ); + expect(argNames('api')).not.toContain('experimentalKnob'); + expect(argNames('api')).not.toContain('experimentalChanged'); + expect(argNames('inputs')).not.toContain('experimentalKnob'); + }); + + it('says why a member is missing from the table', () => { + const debug = vi.fn(); + + extractArgTypesFromData(componentIn(SOURCE), { + metadataJson: undefined, + propsTable: 'api', + logger: { warn: vi.fn(), debug }, + }); + + expect(debug.mock.calls.map(([message]) => message)).toContain( + "PropsTableComponent.cdr left out of the props table: propsTable 'api'" + ); + }); + + it('takes an @internal `model()` and its synthesized change output together', () => { + expect(argNames('all')).toEqual(expect.arrayContaining(['draft', 'draftChange'])); + expect(argNames('api')).not.toContain('draft'); + expect(argNames('api')).not.toContain('draftChange'); + expect(argNames('api')).toEqual(expect.arrayContaining(['published', 'publishedChange'])); + }); + + it('narrows to the inputs section, keeping each documented model pair whole', () => { + const inputs = argNames('inputs'); + + expect(inputs).toEqual(expect.arrayContaining(['density', 'published', 'publishedChange'])); + expect(inputs).not.toContain('helperLabel'); + expect(inputs).not.toContain('densityChange'); + expect(inputs).not.toContain('draft'); + expect(inputs).not.toContain('draftChange'); + }); +}); diff --git a/code/lib/angular-cm/src/analyzer/members.ts b/code/lib/angular-cm/src/analyzer/members.ts index aee759cac81e..eca4ad8e04f4 100644 --- a/code/lib/angular-cm/src/analyzer/members.ts +++ b/code/lib/angular-cm/src/analyzer/members.ts @@ -156,7 +156,10 @@ const visitProperty = ( } const signal = parseSignalCall(ctx, member); if (signal) { - const entry = entryFor(ctx, member, buildSignalEntry(ctx, member, signal)); + const entry = entryFor(ctx, member, { + ...buildSignalEntry(ctx, member, signal), + ...memberApiFields(ctx, member), + }); if (signal.kind !== 'output') { members.inputs.push(entry); } @@ -184,6 +187,7 @@ const buildDecoratorInput = ( optional: config.required !== undefined ? !config.required : !!member.questionToken, ...(config.required === undefined ? {} : { required: config.required }), ...(member.initializer ? { defaultValue: member.initializer.getText() } : {}), + ...memberApiFields(ctx, member), ...getJsDocDescription(ctx.ts, member), ...getJsDocTagsField(ctx.ts, member), }; @@ -199,6 +203,7 @@ const buildDecoratorOutput = ( name: decoratorStringArg(ctx, decorator) ?? memberName(ctx.ts, member.name), ...(type === undefined ? {} : { type }), ...(member.initializer ? { defaultValue: member.initializer.getText() } : {}), + ...memberApiFields(ctx, member), ...getJsDocDescription(ctx.ts, member), ...getJsDocTagsField(ctx.ts, member), }; @@ -216,12 +221,41 @@ const buildPlainProperty = ( ...(type === undefined ? {} : { type }), optional: !!member.questionToken, ...(member.initializer ? { defaultValue: initializerText(ctx.ts, member.initializer) } : {}), + ...memberApiFields(ctx, member), ...getJsDocDescription(ctx.ts, member), ...getJsDocTagsField(ctx.ts, member), ...(names.length ? { decorators: names.map((name) => ({ name })) } : {}), }; }; +const accessibilityOf = ( + ctx: AnalyzerContext, + node: ts.Node +): 'private' | 'protected' | undefined => { + for (const modifier of ctx.ts.getModifiers(node as ts.HasModifiers) ?? []) { + if (modifier.kind === ctx.ts.SyntaxKind.PrivateKeyword) { + return 'private'; + } + if (modifier.kind === ctx.ts.SyntaxKind.ProtectedKeyword) { + return 'protected'; + } + } + return undefined; +}; + +// Several declarations for an accessor pair, whose modifiers and doc comment may sit on either half. +const memberApiFields = ( + ctx: AnalyzerContext, + ...nodes: (ts.Node | undefined)[] +): Pick => { + const declared = nodes.filter((node): node is ts.Node => node !== undefined); + const visibility = declared.map((node) => accessibilityOf(ctx, node)).find(Boolean); + return { + ...(visibility === undefined ? {} : { visibility }), + ...(declared.some((node) => hasJsDocTag(ctx.ts, node, 'internal')) ? { internal: true } : {}), + }; +}; + const isStatic = (ctx: AnalyzerContext, node: ts.Node): boolean => (ctx.ts.getModifiers(node as ts.HasModifiers) ?? []).some( (modifier) => modifier.kind === ctx.ts.SyntaxKind.StaticKeyword @@ -268,6 +302,7 @@ const visitMethod = (ctx: AnalyzerContext, member: ts.MethodDeclaration): Method name: memberName(ctx.ts, member.name), args, returnType, + ...memberApiFields(ctx, member), ...getJsDocDescription(ts, member), ...getJsDocTagsField(ts, member), }; @@ -293,23 +328,15 @@ const visitConstructorProperties = ( ): void => { const { ts } = ctx; for (const parameter of constructor.parameters) { - // Only parameter properties declare a field, and only publicly visible ones belong in the props - // table: the `private readonly` service injections of real projects would otherwise fill it. - const modifiers = (ts.getModifiers(parameter) ?? []).map((modifier) => modifier.kind); - const declaresField = modifiers.some( - (kind) => - kind === ts.SyntaxKind.PublicKeyword || - kind === ts.SyntaxKind.PrivateKeyword || - kind === ts.SyntaxKind.ProtectedKeyword || - kind === ts.SyntaxKind.ReadonlyKeyword - ); - const isHidden = modifiers.some( - (kind) => kind === ts.SyntaxKind.PrivateKeyword || kind === ts.SyntaxKind.ProtectedKeyword + // Only parameter properties declare a field; a plain parameter is no member at all. + const declaresField = (ts.getModifiers(parameter) ?? []).some( + (modifier) => + modifier.kind === ts.SyntaxKind.PublicKeyword || + modifier.kind === ts.SyntaxKind.PrivateKeyword || + modifier.kind === ts.SyntaxKind.ProtectedKeyword || + modifier.kind === ts.SyntaxKind.ReadonlyKeyword ); - if (!declaresField || isHidden) { - if (isHidden) { - dropped(constructor, parameter.name.getText(), 'a private or protected parameter property'); - } + if (!declaresField) { continue; } const type = parameter.type ? ctx.types.render(parameter.type) : ctx.types.infer(parameter); @@ -323,6 +350,7 @@ const visitConstructorProperties = ( ...(parameter.initializer ? { defaultValue: initializerText(ctx.ts, parameter.initializer) } : {}), + ...memberApiFields(ctx, parameter), ...getJsDocDescription(ts, parameter), ...getJsDocTagsField(ts, parameter), }, @@ -364,6 +392,7 @@ const visitAccessorPair = ( ...(getter ? getDecorators(ctx, getter) : []), ...(setter ? getDecorators(ctx, setter) : []), ]; + const apiFields = memberApiFields(ctx, getter, setter); const accessorEntry = (value: T): MemberEntry => ({ declName: name, isStatic: isStatic(ctx, member), @@ -378,6 +407,7 @@ const visitAccessorPair = ( ...(type === undefined ? {} : { type }), optional: config.required !== undefined ? !config.required : false, ...(config.required === undefined ? {} : { required: config.required }), + ...apiFields, ...description, ...tags, }) @@ -390,30 +420,19 @@ const visitAccessorPair = ( accessorEntry({ name: decoratorStringArg(ctx, outputDecorator) ?? name, ...(type === undefined ? {} : { type }), + ...apiFields, ...description, ...tags, }) ); return; } - // Undecorated non-public accessors are implementation detail (host-binding getters, CVA - // plumbing); a props-table row for them is noise. - const nonPublic = [getter, setter].some((accessor) => - (accessor ? (ts.getModifiers(accessor) ?? []) : []).some( - (modifier) => - modifier.kind === ts.SyntaxKind.PrivateKeyword || - modifier.kind === ts.SyntaxKind.ProtectedKeyword - ) - ); - if (nonPublic) { - dropped(member, name, 'an undecorated private or protected accessor'); - return; - } members.properties.push( accessorEntry({ name, ...(type === undefined ? {} : { type }), optional: false, + ...apiFields, ...description, ...tags, // The props table routes the view-child and content-child sections off this field, so an diff --git a/code/lib/angular-cm/src/extract-arg-types.ts b/code/lib/angular-cm/src/extract-arg-types.ts index 849f73085eec..feb05aa190ae 100644 --- a/code/lib/angular-cm/src/extract-arg-types.ts +++ b/code/lib/angular-cm/src/extract-arg-types.ts @@ -32,10 +32,25 @@ const NOOP_LOGGER: ParsingLogger = { debug: () => {}, }; +/** + * Which members reach the props table, as a strict ladder: `all` ⊃ `api` ⊃ `inputs`. + * + * - `all`: every member of every section. + * - `api`: the component's template-facing surface, meaning declared inputs and outputs whatever + * their TypeScript visibility, plus every property and method that is not TypeScript-`private`, + * ES-`#`, or carrying a JSDoc `internal` tag. + * - `inputs`: the inputs section, plus the `${name}Change` output a documented `model()` needs for + * its two-way binding to make sense. + * + * The tag is written without its `@` because `stripInternal` deletes any declaration whose leading + * comment contains that literal. + */ +export type PropsTableMode = 'all' | 'api' | 'inputs'; + export interface ExtractArgTypesOptions { metadataJson: MetadataJson | undefined; - /** The `angularFilterNonInputControls` flag, required so no host inherits a silent default. */ - filterNonInputControls: boolean | undefined; + /** Required so no host inherits a silent default. */ + propsTable: PropsTableMode; logger?: ParsingLogger; } @@ -344,6 +359,24 @@ const extractMemberJsDocTags = ( }; }; +// `@internal` declares a member non-API wherever it appears. TypeScript `private` and ES `#` only +// bar access from code and the component's own template; a consuming template still binds an input +// or output whatever its modifier says (Angular honours modifiers only behind the opt-in +// `strictInputAccessModifiers`), so the inputs and outputs sections filter solely on `@internal`. +const documentedInMode = ( + item: Method | Property, + section: string, + propsTable: PropsTableMode +): boolean => { + if (propsTable === 'all') { + return true; + } + if (item.internal === true || item.name.startsWith('#')) { + return false; + } + return section === 'inputs' || section === 'outputs' || item.visibility !== 'private'; +}; + const readMembers = (componentData: Entry, key: string): (Method | Property)[] => ((componentData as unknown as Record)[key] as | (Method | Property)[] @@ -369,12 +402,13 @@ const getModelProperties = (componentData: Entry): Property[] => { export const extractArgTypesFromData = ( componentData: Entry, - { metadataJson, filterNonInputControls, logger = NOOP_LOGGER }: ExtractArgTypesOptions + { metadataJson, propsTable, logger = NOOP_LOGGER }: ExtractArgTypesOptions ) => { const sectionToItems: Record = {}; - const componentClasses: MemberKey[] = filterNonInputControls - ? ['inputsClass'] - : ['propertiesClass', 'methodsClass', 'inputsClass', 'outputsClass']; + const componentClasses: MemberKey[] = + propsTable === 'inputs' + ? ['inputsClass'] + : ['propertiesClass', 'methodsClass', 'inputsClass', 'outputsClass']; const memberKeys: MemberKey[] = isDirectiveEntry(componentData) ? componentClasses : ['properties', 'methods']; @@ -385,11 +419,6 @@ export const extractArgTypesFromData = ( memberKeys.forEach((key: MemberKey) => { const data = readMembers(componentData, key); data.forEach((item: Method | Property) => { - // ES-private `#member`s cannot be bound from outside the class, so their props-table row is - // noise. - if (item.name.startsWith('#')) { - return; - } const section = mapItemToSection(key, item); // A `model()` surfaces as an input plus the `${name}Change` synthesized below, so the @@ -398,6 +427,13 @@ export const extractArgTypesFromData = ( return; } + if (!documentedInMode(item, section, propsTable)) { + logger.debug( + `${componentData.name}.${item.name} left out of the props table: propsTable '${propsTable}'` + ); + return; + } + const defaultValue = isMethod(item) ? undefined : extractDefaultValue(item, logger); const type: SBType = @@ -431,32 +467,35 @@ export const extractArgTypesFromData = ( }); }); - // The `${name}Change` output this shape never carries directly, synthesized after the loop so - // `filterNonInputControls` cannot hide it. - modelProperties.forEach((item) => { - const changeName = `${item.name}Change`; - - // An output rather than the model input it derives from: no `defaultValue`, never required to - // bind, and typed as the emitted-payload handler signature. - const argType = { - name: changeName, - description: item.rawdescription || item.description, - type: { name: 'other', value: 'void' } as SBType, - action: changeName, - table: { - category: 'outputs', - type: { - summary: `(e: ${item.type}) => void`, - required: false, + // The `${name}Change` output this shape never carries directly. It follows its model's input + // row: synthesized even in `inputs` mode, which narrows sections but must not split a documented + // pair, and skipped when the mode hides the model itself. + modelProperties + .filter((item) => documentedInMode(item, 'inputs', propsTable)) + .forEach((item) => { + const changeName = `${item.name}Change`; + + // An output rather than the model input it derives from: no `defaultValue`, never required to + // bind, and typed as the emitted-payload handler signature. + const argType = { + name: changeName, + description: item.rawdescription || item.description, + type: { name: 'other', value: 'void' } as SBType, + action: changeName, + table: { + category: 'outputs', + type: { + summary: `(e: ${item.type}) => void`, + required: false, + }, }, - }, - }; + }; - if (!sectionToItems.outputs) { - sectionToItems.outputs = []; - } - sectionToItems.outputs.push(argType); - }); + if (!sectionToItems.outputs) { + sectionToItems.outputs = []; + } + sectionToItems.outputs.push(argType); + }); const argTypes: ArgTypes = {}; SECTION_ORDER.forEach((section) => { diff --git a/code/lib/angular-cm/src/index.ts b/code/lib/angular-cm/src/index.ts index a14f1febcb3c..a213fb3f3591 100644 --- a/code/lib/angular-cm/src/index.ts +++ b/code/lib/angular-cm/src/index.ts @@ -1,4 +1,4 @@ export { extractArgTypesFromData } from './extract-arg-types.ts'; -export type { ExtractArgTypesOptions, ParsingLogger } from './extract-arg-types.ts'; +export type { ExtractArgTypesOptions, ParsingLogger, PropsTableMode } from './extract-arg-types.ts'; export { AngularComponentMetaManager } from './manager.ts'; export type { AngularClassMeta, AngularComponentMetaResult } from './types.ts'; diff --git a/code/lib/angular-cm/src/types.ts b/code/lib/angular-cm/src/types.ts index 7d57286778c9..5c2b98f6ca7d 100644 --- a/code/lib/angular-cm/src/types.ts +++ b/code/lib/angular-cm/src/types.ts @@ -30,6 +30,10 @@ export interface Method { args: Argument[]; returnType: string; decorators?: Decorator[]; + /** TypeScript accessibility, omitted for a public member. */ + visibility?: 'private' | 'protected'; + /** Whether the member carries a JSDoc `internal` tag. */ + internal?: boolean; description?: Html; rawdescription?: string; jsdoctags?: JsDocTag[]; @@ -38,6 +42,10 @@ export interface Method { export interface Property { name: string; decorators?: Decorator[]; + /** TypeScript accessibility, omitted for a public member. */ + visibility?: 'private' | 'protected'; + /** Whether the member carries a JSDoc `internal` tag. */ + internal?: boolean; /** Omitted for members the analyzer cannot type, e.g. `@HostBinding`. */ type?: string; /** Omitted for `@Input()` properties, emitted for the rest. */ diff --git a/code/lib/angular-compodoc/src/browser.test.ts b/code/lib/angular-compodoc/src/browser.test.ts new file mode 100644 index 000000000000..b36af78f07ae --- /dev/null +++ b/code/lib/angular-compodoc/src/browser.test.ts @@ -0,0 +1,57 @@ +import { afterAll, afterEach, describe, expect, it, vi } from 'vitest'; + +// browser.ts destructures FEATURES from the global once at first import, so the stub must exist +// before that import evaluates; only property mutation on this reference is live afterwards. +const flags = vi.hoisted(() => { + const features = { angularFilterNonInputControls: false }; + vi.stubGlobal('FEATURES', features); + return features; +}); + +import { extractArgTypesFromData, setCompodocJson } from './browser.ts'; + +setCompodocJson({ + components: [], + directives: [], + pipes: [], + injectables: [], + classes: [], + miscellaneous: { typealiases: [], enumerations: [] }, +} as never); + +afterEach(() => { + flags.angularFilterNonInputControls = false; +}); + +afterAll(() => { + vi.unstubAllGlobals(); +}); + +const componentData = { + name: 'ProbeComponent', + type: 'component', + inputsClass: [{ name: 'label', type: 'string', optional: false }], + outputsClass: [], + propertiesClass: [{ name: 'note', type: 'string', optional: false }], + methodsClass: [], +} as never; + +const names = (options?: Parameters[1]) => + Object.keys(extractArgTypesFromData(componentData, options)); + +describe('the per-call option versus the deprecated feature', () => { + it('falls back to the feature when no option is passed', () => { + expect(names()).toEqual(['note', 'label']); + + flags.angularFilterNonInputControls = true; + expect(names()).toEqual(['label']); + }); + + it('lets the option decide when it is passed, whatever the feature says', () => { + flags.angularFilterNonInputControls = true; + expect(names({ filterNonInputControls: false })).toEqual(['note', 'label']); + + flags.angularFilterNonInputControls = false; + expect(names({ filterNonInputControls: true })).toEqual(['label']); + }); +}); diff --git a/code/lib/angular-compodoc/src/browser.ts b/code/lib/angular-compodoc/src/browser.ts index 95135905b16f..5b7c22140aba 100644 --- a/code/lib/angular-compodoc/src/browser.ts +++ b/code/lib/angular-compodoc/src/browser.ts @@ -20,6 +20,18 @@ import { // (including tests) mutate this object between calls, and a missing `FEATURES` must keep throwing. const { FEATURES } = global; +/** + * Who decides which members the props table renders. + * + * `@storybook/angular-vite` supersedes the `angularFilterNonInputControls` feature with a + * `propsTable` framework option, so it passes its own answer rather than letting this adapter read + * a flag that no longer owns the decision there. `@storybook/angular` passes nothing and keeps the + * feature. + */ +export interface CompodocExtractOptions { + filterNonInputControls?: boolean; +} + export { checkValidCompodocJson, checkValidComponentOrDirective, @@ -41,20 +53,24 @@ const unwrapHtml = (html: unknown): string => new global.DOMParser().parseFromString(html as string, 'text/html').body.textContent ?? ''; export const extractArgTypesFromData = ( - componentData: Parameters[0] + componentData: Parameters[0], + // Asserted rather than optional-chained: a preview without `FEATURES` is broken, and this has + // always thrown there rather than silently reading the flag as `false`. + { filterNonInputControls = FEATURES!.angularFilterNonInputControls }: CompodocExtractOptions = {} ) => extractArgTypesFromDataShared(componentData, { compodocJson: getCompodocJson(), - // Asserted rather than optional-chained: a preview without `FEATURES` is broken, and this has - // always thrown there rather than silently reading the flag as `false`. - filterNonInputControls: FEATURES!.angularFilterNonInputControls, + filterNonInputControls, logger, unwrapHtml, }); -export const extractArgTypes = (component: Component | Directive) => { +export const extractArgTypes = ( + component: Component | Directive, + options?: CompodocExtractOptions +) => { const componentData = getComponentData(component, { compodocJson: getCompodocJson(), logger }); - return componentData && extractArgTypesFromData(componentData); + return componentData && extractArgTypesFromData(componentData, options); }; export const extractComponentDescription = (component: Component | Directive) => { diff --git a/code/lib/docgen-harness/README.md b/code/lib/docgen-harness/README.md index 70d21579599a..7a1876483fe9 100644 --- a/code/lib/docgen-harness/README.md +++ b/code/lib/docgen-harness/README.md @@ -182,7 +182,7 @@ Each has a red marker in `vue3-legacy-gaps.test.ts`. - #9721 -> `jsdoc-tags/`: member JSDoc tags must reach `table.jsDocTags` structurally. Red marker. - #33779 (not reproduced) -> `decorator-union-enum/`: the reported union collapse does not occur at compodoc 2.0.0; regression baseline, no marker. - #29697 (not reproduced) -> `signal-io/`: aliased signal inputs record under their alias at 2.0.0; regression baseline, no marker. -- #22007 -> `properties-methods-noise/`: the filter flag's origin case, and the fixture where both flag states meaningfully differ. +- #22007 -> `properties-methods-noise/`: the filter flag's origin case, and the fixture where both flag states meaningfully differ. The ACM engine closes it: `propsTable: 'api'` (its default) drops private and `#` properties and methods plus `@internal` members, while keeping `protected` members and every declared input and output, so the `acm-` baselines record fewer rows than the legacy ones on purpose. ## The performance bench diff --git a/code/lib/docgen-harness/src/angular/__testfixtures__/decorator-getter-setter/acm-argtypes.snapshot b/code/lib/docgen-harness/src/angular/__testfixtures__/decorator-getter-setter/acm-argtypes.snapshot index 5cbdf4066d99..7f088f883875 100644 --- a/code/lib/docgen-harness/src/angular/__testfixtures__/decorator-getter-setter/acm-argtypes.snapshot +++ b/code/lib/docgen-harness/src/angular/__testfixtures__/decorator-getter-setter/acm-argtypes.snapshot @@ -1,21 +1,4 @@ { - "innerVolume": { - "description": undefined, - "name": "innerVolume", - "table": { - "category": "properties", - "defaultValue": { - "summary": 5, - }, - "type": { - "required": true, - "summary": "number", - }, - }, - "type": { - "name": "number", - }, - }, "volume": { "description": "Playback volume, clamped between 0 and 10.", "name": "volume", diff --git a/code/lib/docgen-harness/src/angular/__testfixtures__/properties-methods-noise/acm-argtypes-filtered.snapshot b/code/lib/docgen-harness/src/angular/__testfixtures__/properties-methods-noise/acm-argtypes-filtered.snapshot index b988efd75ff0..283cee1f55a9 100644 --- a/code/lib/docgen-harness/src/angular/__testfixtures__/properties-methods-noise/acm-argtypes-filtered.snapshot +++ b/code/lib/docgen-harness/src/angular/__testfixtures__/properties-methods-noise/acm-argtypes-filtered.snapshot @@ -1,4 +1,21 @@ { + "density": { + "description": undefined, + "name": "density", + "table": { + "category": "inputs", + "defaultValue": { + "summary": "compact", + }, + "type": { + "required": true, + "summary": "string", + }, + }, + "type": { + "name": "string", + }, + }, "title": { "description": undefined, "name": "title", diff --git a/code/lib/docgen-harness/src/angular/__testfixtures__/properties-methods-noise/acm-argtypes.snapshot b/code/lib/docgen-harness/src/angular/__testfixtures__/properties-methods-noise/acm-argtypes.snapshot index 563768817f55..6237dae0a615 100644 --- a/code/lib/docgen-harness/src/angular/__testfixtures__/properties-methods-noise/acm-argtypes.snapshot +++ b/code/lib/docgen-harness/src/angular/__testfixtures__/properties-methods-noise/acm-argtypes.snapshot @@ -1,4 +1,22 @@ { + "clampPage": { + "description": undefined, + "name": "clampPage", + "table": { + "category": "methods", + "defaultValue": { + "summary": undefined, + }, + "type": { + "required": false, + "summary": "() => void", + }, + }, + "type": { + "name": "other", + "value": "void", + }, + }, "currentPage": { "description": undefined, "name": "currentPage", @@ -16,6 +34,77 @@ "name": "number", }, }, + "density": { + "description": undefined, + "name": "density", + "table": { + "category": "inputs", + "defaultValue": { + "summary": "compact", + }, + "type": { + "required": true, + "summary": "string", + }, + }, + "type": { + "name": "string", + }, + }, + "densityChange": { + "action": "densityChange", + "description": undefined, + "name": "densityChange", + "table": { + "category": "outputs", + "defaultValue": { + "summary": undefined, + }, + "type": { + "required": true, + "summary": "EventEmitter", + }, + }, + "type": { + "name": "other", + "value": "void", + }, + }, + "helperLabel": { + "description": undefined, + "name": "helperLabel", + "table": { + "category": "properties", + "defaultValue": { + "summary": "Next page", + }, + "type": { + "required": true, + "summary": "string", + }, + }, + "type": { + "name": "string", + }, + }, + "host": { + "description": undefined, + "name": "host", + "table": { + "category": "properties", + "defaultValue": { + "summary": undefined, + }, + "type": { + "required": true, + "summary": "ElementRef", + }, + }, + "type": { + "name": "other", + "value": "empty-enum", + }, + }, "isActive": { "description": undefined, "name": "isActive", @@ -69,6 +158,23 @@ "value": "void", }, }, + "pageLabel": { + "description": undefined, + "name": "pageLabel", + "table": { + "category": "properties", + "defaultValue": { + "summary": undefined, + }, + "type": { + "required": true, + "summary": "string", + }, + }, + "type": { + "name": "string", + }, + }, "panel": { "description": undefined, "name": "panel", diff --git a/code/lib/docgen-harness/src/angular/__testfixtures__/properties-methods-noise/acm-snippet-PropsAsWritten.snapshot b/code/lib/docgen-harness/src/angular/__testfixtures__/properties-methods-noise/acm-snippet-PropsAsWritten.snapshot index 3a7f96202180..53034ceb6b52 100644 --- a/code/lib/docgen-harness/src/angular/__testfixtures__/properties-methods-noise/acm-snippet-PropsAsWritten.snapshot +++ b/code/lib/docgen-harness/src/angular/__testfixtures__/properties-methods-noise/acm-snippet-PropsAsWritten.snapshot @@ -1,3 +1,4 @@ + [title]="'Paged results'" + (densityChange)="densityChange($event)"> \ No newline at end of file diff --git a/code/lib/docgen-harness/src/angular/__testfixtures__/properties-methods-noise/argtypes-filtered.snapshot b/code/lib/docgen-harness/src/angular/__testfixtures__/properties-methods-noise/argtypes-filtered.snapshot index 43cf589c305c..7b20b8822257 100644 --- a/code/lib/docgen-harness/src/angular/__testfixtures__/properties-methods-noise/argtypes-filtered.snapshot +++ b/code/lib/docgen-harness/src/angular/__testfixtures__/properties-methods-noise/argtypes-filtered.snapshot @@ -1,4 +1,22 @@ { + "density": { + "description": " +", + "name": "density", + "table": { + "category": "inputs", + "defaultValue": { + "summary": "compact", + }, + "type": { + "required": true, + "summary": "string", + }, + }, + "type": { + "name": "string", + }, + }, "title": { "description": " ", diff --git a/code/lib/docgen-harness/src/angular/__testfixtures__/properties-methods-noise/argtypes.snapshot b/code/lib/docgen-harness/src/angular/__testfixtures__/properties-methods-noise/argtypes.snapshot index b898745bf97b..d0561853d5b4 100644 --- a/code/lib/docgen-harness/src/angular/__testfixtures__/properties-methods-noise/argtypes.snapshot +++ b/code/lib/docgen-harness/src/angular/__testfixtures__/properties-methods-noise/argtypes.snapshot @@ -17,6 +17,42 @@ "name": "string", }, }, + "buildId": { + "description": "", + "name": "buildId", + "table": { + "category": "properties", + "defaultValue": { + "summary": "noise-1", + }, + "type": { + "required": true, + "summary": "string", + }, + }, + "type": { + "name": "string", + }, + }, + "clampPage": { + "description": " +", + "name": "clampPage", + "table": { + "category": "methods", + "defaultValue": { + "summary": undefined, + }, + "type": { + "required": false, + "summary": "() => void", + }, + }, + "type": { + "name": "other", + "value": "void", + }, + }, "currentPage": { "description": " ", @@ -35,6 +71,61 @@ "name": "number", }, }, + "density": { + "description": " +", + "name": "density", + "table": { + "category": "inputs", + "defaultValue": { + "summary": "compact", + }, + "type": { + "required": true, + "summary": "string", + }, + }, + "type": { + "name": "string", + }, + }, + "densityChange": { + "action": "densityChange", + "description": undefined, + "name": "densityChange", + "table": { + "category": "outputs", + "defaultValue": { + "summary": undefined, + }, + "type": { + "required": true, + "summary": "EventEmitter", + }, + }, + "type": { + "name": "other", + "value": "void", + }, + }, + "helperLabel": { + "description": " +", + "name": "helperLabel", + "table": { + "category": "properties", + "defaultValue": { + "summary": "Next page", + }, + "type": { + "required": true, + "summary": "string", + }, + }, + "type": { + "name": "string", + }, + }, "isActive": { "description": " ", @@ -73,6 +164,25 @@ "value": "empty-enum", }, }, + "markDirty": { + "description": " +", + "name": "markDirty", + "table": { + "category": "methods", + "defaultValue": { + "summary": undefined, + }, + "type": { + "required": false, + "summary": "() => void", + }, + }, + "type": { + "name": "other", + "value": "void", + }, + }, "nextPage": { "description": " ", @@ -92,6 +202,24 @@ "value": "void", }, }, + "pageCount": { + "description": " +", + "name": "pageCount", + "table": { + "category": "properties", + "defaultValue": { + "summary": 10, + }, + "type": { + "required": true, + "summary": "number", + }, + }, + "type": { + "name": "number", + }, + }, "panel": { "description": " ", @@ -111,6 +239,24 @@ "value": "void", }, }, + "resetPage": { + "description": "", + "name": "resetPage", + "table": { + "category": "methods", + "defaultValue": { + "summary": undefined, + }, + "type": { + "required": false, + "summary": "() => void", + }, + }, + "type": { + "name": "other", + "value": "void", + }, + }, "title": { "description": " ", diff --git a/code/lib/docgen-harness/src/angular/__testfixtures__/properties-methods-noise/compodoc-input.json b/code/lib/docgen-harness/src/angular/__testfixtures__/properties-methods-noise/compodoc-input.json index cad4473cdbb6..fc0078b60f93 100644 --- a/code/lib/docgen-harness/src/angular/__testfixtures__/properties-methods-noise/compodoc-input.json +++ b/code/lib/docgen-harness/src/angular/__testfixtures__/properties-methods-noise/compodoc-input.json @@ -9,7 +9,7 @@ "components": [ { "name": "PropertiesMethodsNoiseComponent", - "id": "component-PropertiesMethodsNoiseComponent-6d8f11599bef17c0ba839c2f891b8720044f22ef8a5893157453b76b86f6e04164eeaa2818fb2a8839dfbfd343973f7a9587c7f57c82c545c7f842fd748fdaec", + "id": "component-PropertiesMethodsNoiseComponent-4f53b3960fa17b257b6e802cf3a85a7cfef453bb3012bcf1ba568c49c03b4a6ce481e8a56deeb65e394ff31c97db229d67ca290fe623b338508526ff5683abc6", "file": "properties-methods-noise.component.ts", "encapsulation": [], "entryComponents": [], @@ -19,12 +19,24 @@ "selector": "sb-properties-methods-noise", "styleUrls": [], "styles": [], - "template": "
{{ title }} {{ currentPage }}
", + "template": "
{{ title }} {{ currentPage }} {{ helperLabel }} {{ pageLabel }}
", "templateUrl": [], "templateVariables": [], "viewProviders": [], "hostDirectives": [], "inputsClass": [ + { + "coverageIgnore": false, + "name": "density", + "defaultValue": "'compact'", + "deprecated": false, + "deprecationMessage": "", + "rawdescription": "\n", + "description": "", + "line": 30, + "type": "string", + "decorators": [] + }, { "coverageIgnore": false, "name": "title", @@ -33,12 +45,22 @@ "deprecationMessage": "", "rawdescription": "\n", "description": "", - "line": 9, + "line": 26, "type": "string", "decorators": [] } ], - "outputsClass": [], + "outputsClass": [ + { + "name": "densityChange", + "coverageIgnore": false, + "defaultValue": "new EventEmitter()", + "deprecated": false, + "deprecationMessage": "", + "line": 32, + "type": "EventEmitter" + } + ], "propertiesClass": [ { "name": "#secret", @@ -50,8 +72,43 @@ "indexKey": "", "optional": false, "description": "", - "line": 13, - "rawdescription": "\n" + "line": 36, + "rawdescription": "\n", + "modifierKind": [123] + }, + { + "name": "buildId", + "coverageIgnore": false, + "defaultValue": "'noise-1'", + "deprecated": false, + "deprecationMessage": "", + "type": "string", + "indexKey": "", + "optional": false, + "description": "", + "line": 51, + "rawdescription": "", + "jsdoctags": [ + { + "pos": 1250, + "end": 1260, + "kind": 328, + "id": 0, + "flags": 16842752, + "modifierFlagsCache": 0, + "transformFlags": 0, + "tagName": { + "pos": 1251, + "end": 1259, + "kind": 80, + "id": 0, + "flags": 16842752, + "transformFlags": 0, + "escapedText": "internal" + }, + "comment": "" + } + ] }, { "name": "currentPage", @@ -63,9 +120,23 @@ "indexKey": "", "optional": false, "description": "", - "line": 11, + "line": 34, "rawdescription": "\n" }, + { + "name": "helperLabel", + "coverageIgnore": false, + "defaultValue": "'Next page'", + "deprecated": false, + "deprecationMessage": "", + "type": "string", + "indexKey": "", + "optional": false, + "description": "", + "line": 40, + "rawdescription": "\n", + "modifierKind": [124] + }, { "name": "isActive", "coverageIgnore": false, @@ -76,7 +147,7 @@ "indexKey": "", "optional": false, "description": "", - "line": 15, + "line": 57, "rawdescription": "\n", "decorators": [ { @@ -96,8 +167,23 @@ "indexKey": "", "optional": false, "description": "", - "line": 15, - "rawdescription": "\n" + "line": 53, + "rawdescription": "\n", + "modifierKind": [148] + }, + { + "name": "pageCount", + "coverageIgnore": false, + "defaultValue": "10", + "deprecated": false, + "deprecationMessage": "", + "type": "number", + "indexKey": "", + "optional": false, + "description": "", + "line": 38, + "rawdescription": "\n", + "modifierKind": [123] }, { "name": "panel", @@ -108,7 +194,7 @@ "indexKey": "", "optional": true, "description": "", - "line": 13, + "line": 55, "rawdescription": "\n", "decorators": [ { @@ -120,6 +206,34 @@ } ], "methodsClass": [ + { + "name": "clampPage", + "coverageIgnore": false, + "args": [], + "optional": false, + "returnType": "void", + "typeParameters": [], + "line": 63, + "deprecated": false, + "deprecationMessage": "", + "rawdescription": "\n", + "description": "", + "modifierKind": [124] + }, + { + "name": "markDirty", + "coverageIgnore": false, + "args": [], + "optional": false, + "returnType": "void", + "typeParameters": [], + "line": 67, + "deprecated": false, + "deprecationMessage": "", + "rawdescription": "\n", + "description": "", + "modifierKind": [123] + }, { "name": "nextPage", "coverageIgnore": false, @@ -127,11 +241,25 @@ "optional": false, "returnType": "void", "typeParameters": [], - "line": 17, + "line": 59, "deprecated": false, "deprecationMessage": "", "rawdescription": "\n", "description": "" + }, + { + "name": "resetPage", + "coverageIgnore": false, + "args": [], + "optional": false, + "returnType": "void", + "typeParameters": [], + "line": 72, + "deprecated": false, + "deprecationMessage": "", + "rawdescription": "", + "description": "", + "jsdoctags": [] } ], "coverageIgnore": false, @@ -147,7 +275,7 @@ "deprecationMessage": "", "rawdescription": "\n", "description": "", - "line": 15, + "line": 57, "type": "boolean", "decorators": [] } @@ -158,11 +286,85 @@ "description": "", "rawdescription": "\n", "type": "component", - "sourceCode": "import type { ElementRef } from '@angular/core';\nimport { Component, HostBinding, Input, ViewChild } from '@angular/core';\n\n@Component({\n selector: 'sb-properties-methods-noise',\n template: '
{{ title }} {{ currentPage }}
',\n})\nexport class PropertiesMethodsNoiseComponent {\n @Input() title = '';\n\n currentPage = 1;\n\n @ViewChild('panel') panel?: ElementRef;\n\n @HostBinding('class.active') isActive = false;\n\n nextPage(): void {\n this.currentPage += 1;\n }\n}\n", + "sourceCode": "import {\n ChangeDetectorRef,\n Component,\n ElementRef,\n EventEmitter,\n HostBinding,\n Inject,\n Input,\n Output,\n ViewChild,\n signal,\n} from '@angular/core';\n\n@Component({\n selector: 'sb-properties-methods-noise',\n template: '
{{ title }} {{ currentPage }} {{ helperLabel }} {{ pageLabel }}
',\n})\nexport class PropertiesMethodsNoiseComponent {\n // The shape the ui5-webcomponents-ngx measurement is about: 149 components, one of these each.\n // Explicit `@Inject` tokens because the JIT render smoke test has no param-type metadata.\n constructor(\n @Inject(ChangeDetectorRef) private readonly cdr: ChangeDetectorRef,\n @Inject(ElementRef) protected readonly host: ElementRef\n ) {}\n\n @Input() title = '';\n\n // Bindable from a parent template: `strictInputAccessModifiers` is off by default, and output\n // access modifiers are never checked at all.\n @Input() private density = 'compact';\n\n @Output() private densityChange = new EventEmitter();\n\n currentPage = 1;\n\n #secret = 'hidden';\n\n private pageCount = 10;\n\n protected helperLabel = 'Next page';\n\n protected get pageLabel(): string {\n return `${this.currentPage}`;\n }\n\n private get secretLabel(): string {\n return this.#secret;\n }\n\n /** @internal */\n buildId = 'noise-1';\n\n readonly loading = signal(false);\n\n @ViewChild('panel') panel?: ElementRef;\n\n @HostBinding('class.active') isActive = false;\n\n nextPage(): void {\n this.currentPage += 1;\n }\n\n protected clampPage(): void {\n this.currentPage = Math.min(this.currentPage, this.pageCount);\n }\n\n private markDirty(): void {\n this.cdr.markForCheck();\n }\n\n /** @internal */\n resetPage(): void {\n this.currentPage = 1;\n }\n}\n", "assetsDirs": [], "styleUrlsData": "", "stylesData": "", + "constructorObj": { + "name": "constructor", + "description": "", + "deprecated": false, + "deprecationMessage": "", + "args": [ + { + "name": "cdr", + "type": "ChangeDetectorRef", + "optional": false, + "dotDotDotToken": false, + "deprecated": false, + "deprecationMessage": "" + }, + { + "name": "host", + "type": "ElementRef", + "optional": false, + "dotDotDotToken": false, + "deprecated": false, + "deprecationMessage": "" + } + ], + "line": 18, + "rawdescription": "\n", + "jsdoctags": [ + { + "name": "cdr", + "type": "ChangeDetectorRef", + "optional": false, + "dotDotDotToken": false, + "deprecated": false, + "deprecationMessage": "", + "tagName": { + "text": "param" + } + }, + { + "name": "host", + "type": "ElementRef", + "optional": false, + "dotDotDotToken": false, + "deprecated": false, + "deprecationMessage": "", + "tagName": { + "text": "param" + } + } + ] + }, "extends": [], + "accessors": { + "pageLabel": { + "name": "pageLabel", + "getSignature": { + "name": "pageLabel", + "type": "string", + "returnType": "string", + "line": 42, + "rawdescription": "\n", + "description": "" + } + }, + "secretLabel": { + "name": "secretLabel", + "getSignature": { + "name": "secretLabel", + "type": "string", + "returnType": "string", + "line": 46, + "rawdescription": "\n", + "description": "" + } + } + }, "relationships": { "incoming": [], "outgoing": [] @@ -186,7 +388,7 @@ "linktype": "component", "name": "PropertiesMethodsNoiseComponent", "coveragePercent": 0, - "coverageCount": "0/7", + "coverageCount": "0/18", "status": "low" } ] diff --git a/code/lib/docgen-harness/src/angular/__testfixtures__/properties-methods-noise/properties-methods-noise.component.ts b/code/lib/docgen-harness/src/angular/__testfixtures__/properties-methods-noise/properties-methods-noise.component.ts index 1260cfcd6134..1729e098d0f5 100644 --- a/code/lib/docgen-harness/src/angular/__testfixtures__/properties-methods-noise/properties-methods-noise.component.ts +++ b/code/lib/docgen-harness/src/angular/__testfixtures__/properties-methods-noise/properties-methods-noise.component.ts @@ -1,17 +1,55 @@ -import type { ElementRef } from '@angular/core'; -import { Component, HostBinding, Input, ViewChild, signal } from '@angular/core'; +import { + ChangeDetectorRef, + Component, + ElementRef, + EventEmitter, + HostBinding, + Inject, + Input, + Output, + ViewChild, + signal, +} from '@angular/core'; @Component({ selector: 'sb-properties-methods-noise', - template: '
{{ title }} {{ currentPage }}
', + template: '
{{ title }} {{ currentPage }} {{ helperLabel }} {{ pageLabel }}
', }) export class PropertiesMethodsNoiseComponent { + // The shape the ui5-webcomponents-ngx measurement is about: 149 components, one of these each. + // Explicit `@Inject` tokens because the JIT render smoke test has no param-type metadata. + constructor( + @Inject(ChangeDetectorRef) private readonly cdr: ChangeDetectorRef, + @Inject(ElementRef) protected readonly host: ElementRef + ) {} + @Input() title = ''; + // Bindable from a parent template: `strictInputAccessModifiers` is off by default, and output + // access modifiers are never checked at all. + @Input() private density = 'compact'; + + @Output() private densityChange = new EventEmitter(); + currentPage = 1; #secret = 'hidden'; + private pageCount = 10; + + protected helperLabel = 'Next page'; + + protected get pageLabel(): string { + return `${this.currentPage}`; + } + + private get secretLabel(): string { + return this.#secret; + } + + /** @internal */ + buildId = 'noise-1'; + readonly loading = signal(false); @ViewChild('panel') panel?: ElementRef; @@ -21,4 +59,17 @@ export class PropertiesMethodsNoiseComponent { nextPage(): void { this.currentPage += 1; } + + protected clampPage(): void { + this.currentPage = Math.min(this.currentPage, this.pageCount); + } + + private markDirty(): void { + this.cdr.markForCheck(); + } + + /** @internal */ + resetPage(): void { + this.currentPage = 1; + } } diff --git a/code/lib/docgen-harness/src/angular/__testfixtures__/properties-methods-noise/server-snippet-PropsAsWritten.snapshot b/code/lib/docgen-harness/src/angular/__testfixtures__/properties-methods-noise/server-snippet-PropsAsWritten.snapshot index 7ad848494271..b9865c07e473 100644 --- a/code/lib/docgen-harness/src/angular/__testfixtures__/properties-methods-noise/server-snippet-PropsAsWritten.snapshot +++ b/code/lib/docgen-harness/src/angular/__testfixtures__/properties-methods-noise/server-snippet-PropsAsWritten.snapshot @@ -6,7 +6,10 @@ import { PropertiesMethodsNoiseComponent } from './properties-methods-noise.comp imports: [PropertiesMethodsNoiseComponent], template: ` + [title]="'Paged results'" + (densityChange)="densityChange($event)"> `, }) -export class DemoComponent {} \ No newline at end of file +export class DemoComponent { + densityChange(event: unknown) {} +} \ No newline at end of file diff --git a/code/lib/docgen-harness/src/angular/__testfixtures__/properties-methods-noise/snippet-PropsAsWritten.snapshot b/code/lib/docgen-harness/src/angular/__testfixtures__/properties-methods-noise/snippet-PropsAsWritten.snapshot index 3a7f96202180..53034ceb6b52 100644 --- a/code/lib/docgen-harness/src/angular/__testfixtures__/properties-methods-noise/snippet-PropsAsWritten.snapshot +++ b/code/lib/docgen-harness/src/angular/__testfixtures__/properties-methods-noise/snippet-PropsAsWritten.snapshot @@ -1,3 +1,4 @@ + [title]="'Paged results'" + (densityChange)="densityChange($event)"> \ No newline at end of file diff --git a/code/lib/docgen-harness/src/angular/angular-component-meta-baselines.test.ts b/code/lib/docgen-harness/src/angular/angular-component-meta-baselines.test.ts index 95f7d17ed560..4cc284bf2e37 100644 --- a/code/lib/docgen-harness/src/angular/angular-component-meta-baselines.test.ts +++ b/code/lib/docgen-harness/src/angular/angular-component-meta-baselines.test.ts @@ -8,8 +8,11 @@ import { afterAll, describe, expect, it } from 'vitest'; import ts from 'typescript'; +import type { PropsTableMode } from '@storybook/angular-cm'; import { AngularComponentMetaManager, extractArgTypesFromData } from '@storybook/angular-cm'; import type { StrictArgTypes } from '../../../../core/src/csf/story.ts'; +import { expectCurrentOrBetter } from '../compare/expect-current-or-better.ts'; +import { parseArgTypesSnapshot } from '../compare/parse-snapshot.ts'; import { recordArgTypesSnapshot } from '../compare/record-argtypes-snapshot.ts'; import { BASELINE_PATH } from './baseline-path.ts'; import { attachAotCmp, recordSnippets } from './render-helpers.ts'; @@ -52,19 +55,35 @@ describe('angular component-meta baselines', () => { expect(result, `extractComponentMeta found no '${componentExportName}'`).toBeDefined(); const { entry, json } = result!; - const recordArgTypes = async (filterNonInputControls: boolean, prefix: string) => { - // The same call the docgen worker makes, so the recorded baselines represent production - // output. - const extracted = extractArgTypesFromData(entry, { - metadataJson: json, - filterNonInputControls, - }) as StrictArgTypes; + // The same calls the docgen worker makes, so the recorded baselines represent production + // output: `api` is the default and `inputs` is what the deprecated flag maps onto. + const extract = (propsTable: PropsTableMode) => + extractArgTypesFromData(entry, { metadataJson: json, propsTable }) as StrictArgTypes; - const legacyLabel = `${fixtureCase}/${prefix}.snapshot`; + const legacyGate = (prefix: string) => { + const label = `${fixtureCase}/${prefix}.snapshot`; // Asserted to exist so deleting the legacy files can never silently disarm the parity gate. - const committedLegacy = readCommitted(join(testDir, `${prefix}.snapshot`)); - expect(committedLegacy, `missing legacy ${legacyLabel}`).toBeDefined(); + const committed = readCommitted(join(testDir, `${prefix}.snapshot`)); + expect(committed, `missing legacy ${label}`).toBeDefined(); + return { committed: committed!, label, legacyBaseline: true as const }; + }; + + // The committed Compodoc capture is unfiltered, so `all` is the mode that can be held to it. + // Holding `api` to it instead would need a list of the members `api` drops on purpose, and a + // list derived from the engine under test is how a real regression waives itself. + const unfiltered = legacyGate('argtypes'); + expectCurrentOrBetter({ + kind: 'argTypes', + baseline: parseArgTypesSnapshot(unfiltered.committed, unfiltered.label), + candidate: extract('all'), + legacyBaseline: true, + }); + const recordArgTypes = async ( + extracted: StrictArgTypes, + prefix: string, + extraGates: ReturnType[] = [] + ) => { await recordArgTypesSnapshot({ path: join(testDir, `acm-${prefix}.snapshot`), label: `${fixtureCase}/acm-${prefix}.snapshot`, @@ -72,14 +91,18 @@ describe('angular component-meta baselines', () => { // The self-ratchet leg's baseline was written by this same engine, so its table values are // trustworthy enough to gate summary text and required flips too. strictTable: true, - extraGates: [{ committed: committedLegacy!, label: legacyLabel, legacyBaseline: true }], + extraGates, }); return extracted; }; - const argTypes = await recordArgTypes(false, 'argtypes'); - await recordArgTypes(true, 'argtypes-filtered'); + const argTypes = await recordArgTypes(extract('api'), 'argtypes'); + // The legacy inputs-only capture still holds `inputs`: beyond narrowing the sections it only + // drops `@internal` members, and no fixture declares an `@internal` input. + await recordArgTypes(extract('inputs'), 'argtypes-filtered', [ + legacyGate('argtypes-filtered'), + ]); const storiesModule = await import(`./__testfixtures__/${fixtureCase}/input.stories.ts`); const { default: meta, ...stories } = storiesModule; @@ -93,4 +116,43 @@ describe('angular component-meta baselines', () => { // @angular/core types), which can outrun the 10s default on CI. 30_000 ); + + it('drops what no template can bind and keeps what one can', () => { + const testDir = join(fixturesDir, 'properties-methods-noise'); + const result = manager.extractComponentMeta( + join(testDir, 'properties-methods-noise.component.ts'), + { exportName: 'PropertiesMethodsNoiseComponent' } + ); + const argNames = (propsTable: PropsTableMode) => + Object.keys( + extractArgTypesFromData(result!.entry, { metadataJson: result!.json, propsTable }) + ); + + expect(argNames('all')).toEqual( + expect.arrayContaining(['cdr', 'pageCount', 'markDirty', 'secretLabel', '#secret']) + ); + + // The component's own template reads `protected`, so removing it would delete real API. A + // declared input or output is API whatever its accessibility says, because Angular binds it + // from a parent template regardless. + expect(argNames('api')).toEqual( + expect.arrayContaining([ + 'helperLabel', + 'clampPage', + 'pageLabel', + 'host', + 'density', + 'densityChange', + ]) + ); + + expect(argNames('api')).not.toContain('cdr'); + expect(argNames('api')).not.toContain('pageCount'); + expect(argNames('api')).not.toContain('markDirty'); + expect(argNames('api')).not.toContain('secretLabel'); + expect(argNames('api')).not.toContain('#secret'); + expect(argNames('api')).not.toContain('buildId'); + expect(argNames('api')).not.toContain('resetPage'); + expect(argNames('api')).toEqual(expect.arrayContaining(['title', 'currentPage', 'nextPage'])); + }, 30_000); }); diff --git a/code/lib/docgen-harness/src/angular/angular-props-table-render-path.test.ts b/code/lib/docgen-harness/src/angular/angular-props-table-render-path.test.ts new file mode 100644 index 000000000000..a59d64683a72 --- /dev/null +++ b/code/lib/docgen-harness/src/angular/angular-props-table-render-path.test.ts @@ -0,0 +1,82 @@ +// The docgen-server render path for the props table: the worker payload is filtered by +// `propsTable`, but the UI unions the client-side `customArgTypes` back on top of it +// (`mergeServiceArgTypes`), and `customArgTypes` is fed by `parameters.docs.extractArgTypes`. +// Every other test stops at `extractArgTypesFromData`; this one gates the seam where an unfiltered +// Compodoc extraction would resurrect the filtered members in the rendered table. +import { readFileSync } from 'node:fs'; +import { join } from 'node:path'; + +import { afterAll, describe, expect, it, vi } from 'vitest'; + +// config.ts reads FEATURES through @storybook/global at call time, but the compodoc adapter in its +// import graph destructures FEATURES at first import, so the stub must exist before that. +const flags = vi.hoisted(() => { + const features = { angularFilterNonInputControls: false, experimentalDocgenServer: false }; + vi.stubGlobal('FEATURES', features); + return features; +}); + +import type { StrictArgTypes } from '../../../../core/src/csf/story.ts'; +import { mergeServiceArgTypes } from '../../../../core/src/docs-tools/argTypes/docgenServiceArgTypes.ts'; +import { setCompodocJson } from '../../../../frameworks/angular-vite/src/client/compodoc.ts'; +import { parameters } from '../../../../frameworks/angular-vite/src/client/config.ts'; +import { parseArgTypesSnapshot } from '../compare/parse-snapshot.ts'; +import { DecoratorGetterSetterComponent } from './__testfixtures__/decorator-getter-setter/decorator-getter-setter.component.ts'; +import { fixturesDir } from './snippet-recorder.ts'; + +const testDir = join(fixturesDir, 'decorator-getter-setter'); + +setCompodocJson(JSON.parse(readFileSync(join(testDir, 'compodoc-input.json'), 'utf8'))); + +// The committed `api`-mode recording of the same fixture: `volume` without its private backing +// field, exactly what the worker ships as `payload.argTypes`. +const payloadArgTypes = parseArgTypesSnapshot( + readFileSync(join(testDir, 'acm-argtypes.snapshot'), 'utf8'), + 'decorator-getter-setter/acm-argtypes.snapshot' +); + +const extractArgTypes = parameters.docs.extractArgTypes as ( + component: unknown +) => StrictArgTypes | null; + +const mergedWith = (customArgTypes: StrictArgTypes) => + mergeServiceArgTypes({ + payload: { argTypes: payloadArgTypes } as never, + storyId: 'example-decorator--basic', + parameters: {}, + initialArgs: {}, + customArgTypes, + }); + +afterAll(() => { + vi.unstubAllGlobals(); +}); + +describe('parameters.docs.extractArgTypes under the docgen server', () => { + it('extracts through Compodoc when the feature is off', () => { + flags.experimentalDocgenServer = false; + + expect(Object.keys(extractArgTypes(DecoratorGetterSetterComponent)!)).toEqual( + expect.arrayContaining(['innerVolume', 'volume']) + ); + }); + + it('contributes nothing when the worker payload owns extraction, but stays defined', () => { + flags.experimentalDocgenServer = true; + + expect(extractArgTypes(DecoratorGetterSetterComponent)).toEqual({}); + }); + + it('keeps a filtered member out of the merged table, which an unfiltered extraction would resurrect', () => { + expect(payloadArgTypes).not.toHaveProperty('innerVolume'); + + flags.experimentalDocgenServer = false; + const resurrected = mergedWith(extractArgTypes(DecoratorGetterSetterComponent)!); + expect(Object.keys(resurrected)).toContain('innerVolume'); + + flags.experimentalDocgenServer = true; + const merged = mergedWith(extractArgTypes(DecoratorGetterSetterComponent)!); + expect(Object.keys(merged)).toContain('volume'); + expect(Object.keys(merged)).not.toContain('innerVolume'); + }); +}); diff --git a/code/lib/docgen-harness/src/angular/angular-provider-seam.test.ts b/code/lib/docgen-harness/src/angular/angular-provider-seam.test.ts index 66fa48ca31e3..e5676458df9b 100644 --- a/code/lib/docgen-harness/src/angular/angular-provider-seam.test.ts +++ b/code/lib/docgen-harness/src/angular/angular-provider-seam.test.ts @@ -25,9 +25,9 @@ test('angular-vite registers a docgen provider pointing at a worker module that expect(descriptors).toHaveLength(1); expect(isAbsolute(descriptors[0].moduleSpecifier)).toBe(true); expect(existsSync(descriptors[0].moduleSpecifier)).toBe(true); - // The worker receives exactly the Controls filtering flag; the in-process analyzer derives - // everything else from the component files themselves. - expect(descriptors[0].options).toEqual({ angularFilterNonInputControls: true }); + // The worker receives exactly the props-table mode; the in-process analyzer derives everything + // else from the component files themselves. + expect(descriptors[0].options).toEqual({ propsTable: 'inputs' }); }); test('contributes no descriptor when the docgen server feature is off', async () => { diff --git a/code/lib/docgen-harness/src/angular/docgen-fixture.ts b/code/lib/docgen-harness/src/angular/docgen-fixture.ts index 7a65960c8f24..36b2176ce9cf 100644 --- a/code/lib/docgen-harness/src/angular/docgen-fixture.ts +++ b/code/lib/docgen-harness/src/angular/docgen-fixture.ts @@ -17,7 +17,12 @@ export function createFixtureDocgen() { getDocgenPayload: (entry: IndexEntry) => async (): Promise => buildDocgenPayload( { entry }, - { manager, options: {}, logger: noopLogger, resolvePath: (path) => path } + { + manager, + options: { propsTable: 'api' }, + logger: noopLogger, + resolvePath: (path) => path, + } ), dispose: () => manager.dispose(), }; diff --git a/code/lib/docgen-harness/src/compare/argtypes.test.ts b/code/lib/docgen-harness/src/compare/argtypes.test.ts index c8faab8fbfb9..e4c65b65a909 100644 --- a/code/lib/docgen-harness/src/compare/argtypes.test.ts +++ b/code/lib/docgen-harness/src/compare/argtypes.test.ts @@ -14,6 +14,12 @@ describe('compareArgTypes', () => { expect(violations).toEqual([expect.objectContaining({ arg: 'size', kind: 'lost-arg' })]); }); + it('waives a lost ES-private member, which no modern engine is expected to record', () => { + const baseline = argTypes({ '#secret': { name: '#secret', type: { name: 'string' } } }); + + expect(compareArgTypes(baseline, argTypes({}))).toEqual([]); + }); + it('passes when the candidate has keys the baseline lacks', () => { const candidate = argTypes({ size: { name: 'size', type: { name: 'string' } }, diff --git a/code/lib/docgen-harness/src/compare/argtypes.ts b/code/lib/docgen-harness/src/compare/argtypes.ts index 21e9dadda3b8..57e344bfe496 100644 --- a/code/lib/docgen-harness/src/compare/argtypes.ts +++ b/code/lib/docgen-harness/src/compare/argtypes.ts @@ -25,7 +25,8 @@ export function compareArgTypes( const violations: Violation[] = []; for (const [arg, baseEntry] of Object.entries(baseline)) { // ES-private `#member`s are inaccessible outside their class; legacy Compodoc records them - // anyway, and the modern extractor deliberately drops them. Their loss never gates. + // anyway, and the modern extractor only surfaces them under `propsTable: 'all'`. Their loss + // never gates. if (arg.startsWith('#')) { continue; } diff --git a/code/lib/docgen-harness/src/perf/docgen-perf/engines/compodoc-doc.test.ts b/code/lib/docgen-harness/src/perf/docgen-perf/engines/compodoc-doc.test.ts index 363b3f10285a..7ffe38de0c5c 100644 --- a/code/lib/docgen-harness/src/perf/docgen-perf/engines/compodoc-doc.test.ts +++ b/code/lib/docgen-harness/src/perf/docgen-perf/engines/compodoc-doc.test.ts @@ -41,8 +41,10 @@ describe('countDocumentation', () => { }); it('reports none when every member describes itself', () => { - expect(fixture('properties-methods-noise').opaqueTypes).toBe(0); expect(fixture('jsdoc-tags').opaqueTypes).toBe(0); + // properties-methods-noise carries exactly one opaque member: its `@Output()` records + // `EventEmitter` with the payload type dropped, like decorator-io-basics. + expect(fixture('properties-methods-noise').opaqueTypes).toBe(1); }); }); }); diff --git a/code/lib/docgen-harness/tsconfig.json b/code/lib/docgen-harness/tsconfig.json index 3915a6e96c77..b2d4d9bd84af 100644 --- a/code/lib/docgen-harness/tsconfig.json +++ b/code/lib/docgen-harness/tsconfig.json @@ -14,6 +14,7 @@ "exclude": [ "src/angular/angular-baselines.test.ts", "src/angular/angular-component-meta-baselines.test.ts", + "src/angular/angular-props-table-render-path.test.ts", "src/angular/angular-provider-seam.test.ts", "src/angular/angular-render.test.ts", "src/angular/render-helpers.ts", diff --git a/docs/api/main-config/main-config-features.mdx b/docs/api/main-config/main-config-features.mdx index 267209d7b49b..2214702458f7 100644 --- a/docs/api/main-config/main-config-features.mdx +++ b/docs/api/main-config/main-config-features.mdx @@ -107,6 +107,11 @@ Type: `boolean` Filter non-input controls in Angular. +On `@storybook/angular-vite` this feature is deprecated in favor of the [`propsTable` framework option](../../get-started/frameworks/angular-vite.mdx#propstable): `true` maps to `propsTable: 'inputs'` and `false` maps to `propsTable: 'all'`. +The option's third value, `'api'`, needs the `experimentalDocgenServer` feature. +Setting both leaves `propsTable` in charge. +`@storybook/angular` still reads this feature and is unaffected. + ## `argTypeTargetsV7` diff --git a/docs/get-started/frameworks/angular-vite.mdx b/docs/get-started/frameworks/angular-vite.mdx index f6f0da9928b0..878faf17fe00 100644 --- a/docs/get-started/frameworks/angular-vite.mdx +++ b/docs/get-started/frameworks/angular-vite.mdx @@ -439,3 +439,51 @@ Type: `string[]` Default: `['-e', 'json', '-d', '.']` Arguments passed to the `@compodoc/compodoc` CLI when `compodoc` is `true`. The defaults produce a `documentation.json` file in the workspace root. + +#### `propsTable` + +Type: `'all' | 'api' | 'inputs'` + +Default: `'api'` + +Which of your component's members the props table renders. +The three values are a ladder, each one a subset of the one above it: + +| Value | Renders | +| ---------- | ------------------------------------------------------------------------- | +| `'all'` | Every member of every section: properties, inputs, outputs and methods. | +| `'api'` | The same four sections, narrowed to your component's template-facing API. | +| `'inputs'` | The inputs section only. | + +`'api'` keeps every declared input and output, whatever its TypeScript visibility. + +Everywhere else, `'api'` drops TypeScript `private` members, ECMAScript private `#` members, and anything tagged `@internal`. +A `private` property or method cannot be reached from any template, and `@internal` declares a member non-API, so a row for them documents your component's wiring rather than its API. +Injected services are the common case: + +```ts +@Component({ selector: 'my-button', template: '' }) +export class ButtonComponent { + // Kept by 'api': a parent template can bind any declared input. + @Input() private label = ''; + + // Dropped by 'api': no template can reach it. + private readonly cdr = inject(ChangeDetectorRef); + + // Kept by 'api': the component's own template can read protected members. + protected pressed = false; +} +``` + +`protected` members are deliberately kept, because Angular templates can bind them and they are therefore part of what a reader needs to know. + +To drop a single member that `'api'` keeps, tag it `@ignore`: + +```ts +/** @ignore */ +protected internalHelper = 0; +``` + +`'api'` needs the `experimentalDocgenServer` feature. +Without it, Storybook reads your components through Compodoc, whose visibility data Storybook cannot interpret reliably, so only `'all'` and `'inputs'` apply. +Asking for `'api'` then logs a warning rather than silently changing what you see.