Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions code/core/src/csf/story.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type { RemoveIndexSignature, Simplify, UnionToIntersection } from 'type-fest';

import type { ToolbarArgType } from '../toolbar';
import type { SBScalarType, SBType } from './SBType';
import type { CoreTypes } from './core-annotations';

Expand Down Expand Up @@ -144,6 +145,8 @@ export interface InputType {
[key: string]: any;
}

export type { ToolbarArgType };

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not sure about where this should be exported from, but it should be exported.


export interface StrictInputType extends InputType {
name: string;
type?: SBType;
Expand All @@ -165,10 +168,10 @@ export interface Globals {
[name: string]: any;
}
export interface GlobalTypes {
[name: string]: InputType;
[name: string]: ToolbarArgType;
}
Comment on lines 170 to 172

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# First, let's find the file and understand the current state
cd /repo
cat -n code/core/src/csf/story.ts | head -180 | tail -30

Repository: storybookjs/storybook

Length of output: 1033


🏁 Script executed:

# Find InputType and ToolbarArgType definitions
rg -n "type InputType|type ToolbarArgType|interface InputType|interface ToolbarArgType" --type ts -A 3

Repository: storybookjs/storybook

Length of output: 586


🏁 Script executed:

# Find all usages of GlobalTypes
rg -n "GlobalTypes" --type ts -C 2

Repository: storybookjs/storybook

Length of output: 10410


🏁 Script executed:

# Get complete InputType and StrictInputType definitions
rg -n "export.*interface InputType|export.*type InputType|export.*interface StrictInputType|export.*type StrictInputType" --type ts -A 20 | head -100

Repository: storybookjs/storybook

Length of output: 2856


🏁 Script executed:

# Check ToolbarConfig definition
rg -n "type ToolbarConfig|interface ToolbarConfig" --type ts -A 5

Repository: storybookjs/storybook

Length of output: 432


🏁 Script executed:

# Check how normalizeInputTypes handles GlobalTypes
cat -n code/core/src/preview-api/modules/store/csf/normalizeInputTypes.ts

Repository: storybookjs/storybook

Length of output: 1433


🏁 Script executed:

# Search for example globalTypes definitions
rg -n "globalTypes.*=" --type ts -A 3 | head -80

Repository: storybookjs/storybook

Length of output: 4268


🏁 Script executed:

# Search for any migration notes or documentation about this change
fd -e md | xargs rg -l "globalTypes|GlobalTypes" | head -10

Repository: storybookjs/storybook

Length of output: 292


🏁 Script executed:

# Check the example of globalTypes definition
cat -n code/core/template/stories/preview.ts | sed -n '52,70p'

Repository: storybookjs/storybook

Length of output: 759


🏁 Script executed:

# Check the documented snippet for globalTypes
cat docs/_snippets/storybook-preview-configure-globaltypes.md

Repository: storybookjs/storybook

Length of output: 2957


🏁 Script executed:

# Check MIGRATION.md for any notes about GlobalTypes changes
rg -n "GlobalTypes|globalTypes" MIGRATION.md -B 2 -A 2

Repository: storybookjs/storybook

Length of output: 1329


Backward compatibility broken by required toolbar property.

Changing GlobalTypes from InputType to ToolbarArgType adds a required toolbar property, which is a breaking change. While ToolbarArgType is a superset of InputType and includes all existing properties, users with existing globalTypes definitions that omit the toolbar property will see type errors.

All documented examples and template code include toolbar configurations, and ToolbarManager.tsx defensively filters entries by !!globalTypes[id].toolbar, confirming awareness of this constraint. Users will need to add toolbar configuration to all globalTypes entries or adjust their type annotations.

🤖 Prompt for AI Agents
In code/core/src/csf/story.ts around lines 170 to 172, the index signature was
changed to ToolbarArgType which makes the toolbar property required and breaks
backward compatibility; change the index type to allow both the old InputType
and the new ToolbarArgType (e.g. use InputType | ToolbarArgType or keep
InputType and make toolbar optional on ToolbarArgType) so existing globalTypes
without toolbar do not error, and update any related type exports/tests/comments
accordingly.

export interface StrictGlobalTypes {
[name: string]: StrictInputType;
[name: string]: StrictInputType & ToolbarArgType;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not particularly happy about this one either to be honest. Should GlobalTypes even be using InputType as its bas? How much of InputType does it actually use? defaultValue is marked as deprecated in InputType but is used in ToolbarArgType. Perhaps we should decouple these types?

}

export interface AddonTypes {
Expand Down
7 changes: 4 additions & 3 deletions code/core/src/manager-api/tests/globals.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import type { GlobalsUpdatedPayload, SetGlobalsPayload } from 'storybook/interna

import { EventEmitter } from 'events';

import type { ToolbarArgType } from '../../toolbar';
import { getEventMetadata as _getEventData } from '../lib/events';
import type { ModuleArgs } from '../lib/types';
import type { SubAPI } from '../modules/globals';
Expand Down Expand Up @@ -61,7 +62,7 @@ describe('globals API', () => {

channel.emit(SET_GLOBALS, {
globals: { a: 'b' },
globalTypes: { a: { type: { name: 'string' } } },
globalTypes: { a: { type: { name: 'string' } } as ToolbarArgType },
} satisfies SetGlobalsPayload);
expect(store.getState()).toEqual({
userGlobals: { a: 'b' },
Expand All @@ -88,7 +89,7 @@ describe('globals API', () => {

channel.emit(SET_GLOBALS, {
globals: { a: 'b' },
globalTypes: { a: { type: { name: 'string' } } },
globalTypes: { a: { type: { name: 'string' } } as ToolbarArgType },
} satisfies SetGlobalsPayload);
expect(store.getState()).toEqual({
userGlobals: { a: 'b' },
Expand Down Expand Up @@ -138,7 +139,7 @@ describe('globals API', () => {
getEventMetadata.mockReturnValueOnce({ sourceType: 'external', ref: { id: 'ref' } } as any);
channel.emit(SET_GLOBALS, {
globals: { a: 'b' },
globalTypes: { a: { type: { name: 'string' } } },
globalTypes: { a: { type: { name: 'string' } } as ToolbarArgType },
} satisfies SetGlobalsPayload);
expect(store.getState()).toEqual({
userGlobals: {},
Expand Down
26 changes: 15 additions & 11 deletions code/core/src/preview-api/modules/store/GlobalsStore.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { describe, expect, it, vi } from 'vitest';

import type { ToolbarArgType } from '../../../toolbar';
import { GlobalsStore } from './GlobalsStore';

vi.mock('storybook/internal/client-logger', () => ({
Expand Down Expand Up @@ -33,8 +34,8 @@ describe('GlobalsStore', () => {
arg2: 2,
},
globalTypes: {
arg2: { defaultValue: 'arg2' },
arg3: { defaultValue: { complex: { object: ['type'] } } },
arg2: { defaultValue: 'arg2' } as ToolbarArgType,
arg3: { defaultValue: { complex: { object: ['type'] } } } as ToolbarArgType,
},
});

Expand All @@ -48,7 +49,10 @@ describe('GlobalsStore', () => {

describe('update', () => {
it('changes the global args', () => {
const store = new GlobalsStore({ globals: { foo: 'old' }, globalTypes: { baz: {} } });
const store = new GlobalsStore({
globals: { foo: 'old' },
globalTypes: { baz: {} as ToolbarArgType },
});

store.update({ foo: 'bar' });
expect(store.get()).toEqual({ foo: 'bar' });
Expand All @@ -60,7 +64,7 @@ describe('GlobalsStore', () => {
it('does not merge objects', () => {
const store = new GlobalsStore({
globals: { obj: { foo: 'old' } },
globalTypes: { baz: {} },
globalTypes: { baz: {} as ToolbarArgType },
});

store.update({ obj: { foo: 'bar' } });
Expand All @@ -78,7 +82,7 @@ describe('GlobalsStore', () => {
arg1: 'arg1',
},
globalTypes: {
arg2: { defaultValue: 'arg2' },
arg2: { defaultValue: 'arg2' } as ToolbarArgType,
},
});

Expand All @@ -104,8 +108,8 @@ describe('GlobalsStore', () => {
arg2: 2,
},
globalTypes: {
arg2: { defaultValue: 'arg2' },
arg3: { defaultValue: { complex: { object: ['type'] } } },
arg2: { defaultValue: 'arg2' } as ToolbarArgType,
arg3: { defaultValue: { complex: { object: ['type'] } } } as ToolbarArgType,
},
});

Expand All @@ -126,7 +130,7 @@ describe('GlobalsStore', () => {
arg3: 'arg3',
},
globalTypes: {
arg2: { defaultValue: 'arg2' },
arg2: { defaultValue: 'arg2' } as ToolbarArgType,
},
});

Expand All @@ -144,7 +148,7 @@ describe('GlobalsStore', () => {
arg1: 'arg1',
},
globalTypes: {
arg2: { defaultValue: 'arg2' },
arg2: { defaultValue: 'arg2' } as ToolbarArgType,
},
});
// However undeclared values aren't persisted
Expand All @@ -162,7 +166,7 @@ describe('GlobalsStore', () => {
arg4: 'arg4',
},
globalTypes: {
arg2: { defaultValue: 'arg2' },
arg2: { defaultValue: 'arg2' } as ToolbarArgType,
},
});

Expand All @@ -186,7 +190,7 @@ describe('GlobalsStore', () => {
arg4: 'edited-arg4',
},
globalTypes: {
arg5: { defaultValue: 'edited-arg5' },
arg5: { defaultValue: 'edited-arg5' } as ToolbarArgType,
},
});
// However undeclared values aren't persisted
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type {
NormalizedProjectAnnotations,
ProjectAnnotations,
Renderer,
StrictGlobalTypes,
} from 'storybook/internal/types';

import { inferArgTypes } from '../inferArgTypes';
Expand All @@ -27,7 +28,7 @@ export function normalizeProjectAnnotations<TRenderer extends Renderer>({
}: ProjectAnnotations<TRenderer>): NormalizedProjectAnnotations<TRenderer> {
return {
...(argTypes && { argTypes: normalizeInputTypes(argTypes as ArgTypes) }),
...(globalTypes && { globalTypes: normalizeInputTypes(globalTypes) }),
...(globalTypes && { globalTypes: normalizeInputTypes(globalTypes) as StrictGlobalTypes }),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Find the normalizeInputTypes function definition and check its return type
ast-grep --pattern 'function normalizeInputTypes($$$) {
  $$$
}'

Repository: storybookjs/storybook

Length of output: 47


🏁 Script executed:

# Search for normalizeInputTypes usage and context
rg -n "normalizeInputTypes" --type ts -A 3 -B 3

Repository: storybookjs/storybook

Length of output: 7572


🏁 Script executed:

# Find the StrictGlobalTypes type definition
rg -n "type StrictGlobalTypes" --type ts -A 5

Repository: storybookjs/storybook

Length of output: 47


🏁 Script executed:

# Check the file structure around line 31
cat -n code/core/src/preview-api/modules/store/csf/normalizeProjectAnnotations.ts | sed -n '1,50p'

Repository: storybookjs/storybook

Length of output: 2713


🏁 Script executed:

# Find type definitions for ArgTypes, GlobalTypes, StrictArgTypes, StrictGlobalTypes
rg -n "type (ArgTypes|GlobalTypes|StrictArgTypes|StrictGlobalTypes)" --type ts

Repository: storybookjs/storybook

Length of output: 1033


🏁 Script executed:

# Check the normalizeInputTypes implementation more closely
cat -n code/core/src/preview-api/modules/store/csf/normalizeInputTypes.ts | head -50

Repository: storybookjs/storybook

Length of output: 1433


🏁 Script executed:

# Look at the normalizeInputType function to understand the logic
cat -n code/core/src/preview-api/modules/store/csf/normalizeInputTypes.ts

Repository: storybookjs/storybook

Length of output: 1433


🏁 Script executed:

# Check if there are any type overloads or declarations for normalizeInputTypes
rg -n "declare.*normalizeInputTypes|overload.*normalizeInputTypes" --type ts

Repository: storybookjs/storybook

Length of output: 47


🏁 Script executed:

# Find the type definitions for ArgTypes and GlobalTypes
rg -n "export type ArgTypes|export type GlobalTypes|export type StrictArgTypes|export type StrictGlobalTypes" --type ts -A 1

Repository: storybookjs/storybook

Length of output: 715


🏁 Script executed:

# Also check if there are interface definitions
rg -n "export interface (ArgTypes|GlobalTypes|StrictArgTypes|StrictGlobalTypes)" --type ts

Repository: storybookjs/storybook

Length of output: 468


🏁 Script executed:

# Get the full type definitions
cat -n code/core/src/csf/story.ts | sed -n '164,180p'

Repository: storybookjs/storybook

Length of output: 658


🏁 Script executed:

# Check if GlobalTypes and ArgTypes are structurally compatible
cat -n code/core/src/csf/story.ts | sed -n '164,195p'

Repository: storybookjs/storybook

Length of output: 1230


🏁 Script executed:

# Look at the issue comment in the code - there seems to be a TODO around line 14-17
git log --oneline code/core/src/preview-api/modules/store/csf/normalizeProjectAnnotations.ts | head -20

Repository: storybookjs/storybook

Length of output: 126


🏁 Script executed:

# Find ToolbarArgType definition
rg -n "type ToolbarArgType|interface ToolbarArgType" --type ts -A 3

Repository: storybookjs/storybook

Length of output: 273


🏁 Script executed:

# Check InputType and StrictInputType definitions to understand the difference
rg -n "export type InputType|export type StrictInputType" --type ts -A 2

Repository: storybookjs/storybook

Length of output: 47


🏁 Script executed:

# Search for InputType and StrictInputType definitions more broadly
rg -n "export type InputType|export type StrictInputType|export interface InputType|export interface StrictInputType" --type ts

Repository: storybookjs/storybook

Length of output: 208


🏁 Script executed:

# Check what ToolbarArgType extends/uses
cat -n code/core/src/toolbar/types.ts | head -60

Repository: storybookjs/storybook

Length of output: 1774


🏁 Script executed:

# Check the InputType and StrictInputType definitions
cat -n code/core/src/csf/story.ts | sed -n '111,160p'

Repository: storybookjs/storybook

Length of output: 2159


🏁 Script executed:

# Check what types argTypes and globalTypes have in ProjectAnnotations
rg -n "interface ProjectAnnotations|type ProjectAnnotations" --type ts -A 15

Repository: storybookjs/storybook

Length of output: 2934


🏁 Script executed:

# Find BaseAnnotations to see the types of argTypes and globalTypes
rg -n "interface BaseAnnotations|type BaseAnnotations" --type ts -A 20

Repository: storybookjs/storybook

Length of output: 2452


🏁 Script executed:

# Also check NormalizedProjectAnnotations more fully
cat -n code/core/src/types/modules/story.ts | sed -n '55,65p'

Repository: storybookjs/storybook

Length of output: 534


🏁 Script executed:

# Find argTypes and globalTypes properties in BaseAnnotations/ProjectAnnotations
cat -n code/core/src/csf/story.ts | sed -n '331,405p'

Repository: storybookjs/storybook

Length of output: 3412


🏁 Script executed:

# Check if globalTypes is defined in ProjectAnnotations
cat -n code/core/src/csf/story.ts | sed -n '405,460p'

Repository: storybookjs/storybook

Length of output: 3112


🏁 Script executed:

# Check what properties are actually preserved by normalizeInputType
cat -n code/core/src/preview-api/modules/store/csf/normalizeInputTypes.ts

Repository: storybookjs/storybook

Length of output: 1433


🏁 Script executed:

# Check if there are any related issues or PRs mentioning this
git log --all --oneline --grep="GlobalTypes\|normalizeInputTypes" | head -20

Repository: storybookjs/storybook

Length of output: 147


🏁 Script executed:

# Check the tests to understand the expected behavior
cat -n code/core/src/preview-api/modules/store/csf/normalizeInputTypes.test.ts | sed -n '68,100p'

Repository: storybookjs/storybook

Length of output: 471


Add function overloads to normalizeInputTypes to enable proper type narrowing.

The type assertion as StrictGlobalTypes is necessary because normalizeInputTypes returns a union type StrictArgTypes | StrictGlobalTypes without overloads to narrow it based on the input. The function should declare overloads to automatically provide the correct return type:

  • normalizeInputTypes(ArgTypes)StrictArgTypes
  • normalizeInputTypes(GlobalTypes)StrictGlobalTypes

This would eliminate the need for the output cast and improve type safety throughout the codebase.

🤖 Prompt for AI Agents
In code/core/src/preview-api/modules/store/csf/normalizeProjectAnnotations.ts
around line 31, the call uses a cast to StrictGlobalTypes because
normalizeInputTypes currently returns a union (StrictArgTypes |
StrictGlobalTypes); add TypeScript overloads to normalizeInputTypes so that
calling it with ArgTypes returns StrictArgTypes and calling it with GlobalTypes
returns StrictGlobalTypes, then remove the cast here; implement two function
signatures (one accepting ArgTypes and returning StrictArgTypes, another
accepting GlobalTypes and returning StrictGlobalTypes) followed by the single
implementation that handles both inputs and returns the properly narrowed type.

decorators: normalizeArrays(decorators),
loaders: normalizeArrays(loaders),
beforeEach: normalizeArrays(beforeEach),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import type {
StoryAnnotationsOrFn as Story,
} from 'storybook/internal/types';

import type { ToolbarArgType } from '../../../../toolbar';
import * as defaultExportAnnotations from './__mocks__/defaultExportAnnotations.mockfile';
import * as namedExportAnnotations from './__mocks__/namedExportAnnotations.mockfile';
import { composeStories, composeStory, setProjectAnnotations } from './portable-stories';
Expand Down Expand Up @@ -102,7 +103,7 @@ describe('composeStory', () => {
const projectAnnotations = {
parameters: { injected: true },
globalTypes: {
locale: { defaultValue: 'en' },
locale: { defaultValue: 'en' } as ToolbarArgType,
},
decorators: [decoratorFromProjectAnnotations],
tags: ['projectTag'],
Expand Down Expand Up @@ -235,7 +236,7 @@ describe('composeStory', () => {
theme: {
name: 'Theme',
defaultValue: 'light',
},
} as ToolbarArgType,
},
};

Expand Down
1 change: 1 addition & 0 deletions code/core/src/types/modules/csf.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ export type {
StrictGlobalTypes,
StrictInputType,
Tag,
ToolbarArgType,
} from 'storybook/internal/csf';

type OrString<T extends string> = T | (string & {});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
import { cleanup, render, screen } from '@testing-library/svelte';
import { afterEach, describe, expect, it } from 'vitest';

import type { ToolbarArgType } from 'storybook/internal/csf';

// import '@testing-library/svelte/vitest';
import { expectTypeOf } from 'expect-type';

Expand Down Expand Up @@ -70,7 +72,7 @@ describe('projectAnnotations', () => {
{
parameters: { injected: true },
globalTypes: {
locale: { defaultValue: 'en' },
locale: { defaultValue: 'en' } as ToolbarArgType,
},
},
]);
Expand Down
Loading