diff --git a/CHANGELOG.prerelease.md b/CHANGELOG.prerelease.md index dd71a1667da1..b622e4db4249 100644 --- a/CHANGELOG.prerelease.md +++ b/CHANGELOG.prerelease.md @@ -1,3 +1,14 @@ +## 10.4.0-alpha.17 + +- Agentic Setup: Keep sample content if users want onboarding - [#34704](https://github.com/storybookjs/storybook/pull/34704), thanks @Sidnioulz! +- Fix ArgsTable borders not visible in Windows High Contrast Mode - [#34264](https://github.com/storybookjs/storybook/pull/34264), thanks @TheSeydiCharyyev! +- MDX: Replace `@storybook/docs-mdx` with inline implementation - [#34611](https://github.com/storybookjs/storybook/pull/34611), thanks @copilot-swe-agent! +- Maintenance: Enhance ghost stories internal tests - [#34707](https://github.com/storybookjs/storybook/pull/34707), thanks @yannbf! +- ReactNative: Add Metro config AST codemod for init - [#34660](https://github.com/storybookjs/storybook/pull/34660), thanks @ndelangen! +- ReactNative: New init setup - [#34665](https://github.com/storybookjs/storybook/pull/34665), thanks @ndelangen! +- Tanstack: Add `@storybook/tanstack-react` package - [#34403](https://github.com/storybookjs/storybook/pull/34403), thanks @huang-julien! +- UI: Fix showing and hiding copy prompt in the correct scenarios - [#34706](https://github.com/storybookjs/storybook/pull/34706), thanks @yannbf! + ## 10.4.0-alpha.16 - Automigration: Move RN on-device addons to `deviceAddons` - [#34659](https://github.com/storybookjs/storybook/pull/34659), thanks @ndelangen! diff --git a/code/addons/docs/src/blocks/components/ArgsTable/ArgsTable.tsx b/code/addons/docs/src/blocks/components/ArgsTable/ArgsTable.tsx index 1f1dfc4394a5..11e0325d6460 100644 --- a/code/addons/docs/src/blocks/components/ArgsTable/ArgsTable.tsx +++ b/code/addons/docs/src/blocks/components/ArgsTable/ArgsTable.tsx @@ -160,6 +160,17 @@ export const TableWrapper = styled.table<{ }, }), }, + // High contrast mode: ensure borders are visible with system colors + '@media (forced-colors: active)': { + tbody: { + filter: 'none', + + '> tr > *': { + borderColor: 'CanvasText', + }, + }, + }, + // End awesome table styling }, })); diff --git a/code/builders/builder-vite/src/change-detection-adapter/index.test.ts b/code/builders/builder-vite/src/change-detection-adapter/index.test.ts new file mode 100644 index 000000000000..e81ad59504cb --- /dev/null +++ b/code/builders/builder-vite/src/change-detection-adapter/index.test.ts @@ -0,0 +1,143 @@ +// Tests the Vite implementation of ChangeDetectionAdapter — wiring of resolve config +// snapshot and chokidar event normalisation. +import { EventEmitter } from 'node:events'; + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import type { ViteDevServer } from 'vite'; + +import { createViteChangeDetectionAdapter } from './index.ts'; + +vi.mock('vite', { spy: true }); + +interface FakeViteDevServer { + config: { + root: string; + resolve?: { + alias?: unknown; + conditions?: string[]; + tsconfig?: string; + }; + }; + watcher: EventEmitter; +} + +function createFakeServer(overrides: Partial = {}): { + server: ViteDevServer; + watcher: EventEmitter; +} { + const watcher = new EventEmitter(); + const server: FakeViteDevServer = { + config: { + root: '/repo', + ...overrides, + }, + watcher, + }; + return { server: server as unknown as ViteDevServer, watcher }; +} + +describe('createViteChangeDetectionAdapter', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('snapshots projectRoot, alias and conditions from server.config in getResolveConfig()', async () => { + const alias = [{ find: '@', replacement: '/repo/src' }]; + const conditions = ['import', 'module', 'default']; + const { server } = createFakeServer({ + root: '/repo', + resolve: { alias, conditions }, + }); + + const adapter = createViteChangeDetectionAdapter(server); + const config = await adapter.getResolveConfig(); + + expect(config).toEqual({ + projectRoot: '/repo', + alias, + conditions, + }); + }); + + it('forwards chokidar `add` events as kind: "add"', () => { + const { server, watcher } = createFakeServer(); + const adapter = createViteChangeDetectionAdapter(server); + const handler = vi.fn(); + adapter.onFileChange(handler); + + watcher.emit('all', 'add', '/repo/src/A.tsx'); + + expect(handler).toHaveBeenCalledWith({ kind: 'add', path: '/repo/src/A.tsx' }); + }); + + it('forwards chokidar `change` events as kind: "change"', () => { + const { server, watcher } = createFakeServer(); + const adapter = createViteChangeDetectionAdapter(server); + const handler = vi.fn(); + adapter.onFileChange(handler); + + watcher.emit('all', 'change', '/repo/src/A.tsx'); + + expect(handler).toHaveBeenCalledWith({ kind: 'change', path: '/repo/src/A.tsx' }); + }); + + it('forwards chokidar `unlink` events as kind: "unlink"', () => { + const { server, watcher } = createFakeServer(); + const adapter = createViteChangeDetectionAdapter(server); + const handler = vi.fn(); + adapter.onFileChange(handler); + + watcher.emit('all', 'unlink', '/repo/src/A.tsx'); + + expect(handler).toHaveBeenCalledWith({ kind: 'unlink', path: '/repo/src/A.tsx' }); + }); + + it('does NOT forward `addDir`, `unlinkDir`, `ready`, `raw`, or `error` chokidar events', () => { + const { server, watcher } = createFakeServer(); + const adapter = createViteChangeDetectionAdapter(server); + const handler = vi.fn(); + adapter.onFileChange(handler); + + watcher.emit('all', 'addDir', '/repo/src/some-dir'); + watcher.emit('all', 'unlinkDir', '/repo/src/some-dir'); + watcher.emit('all', 'ready'); + watcher.emit('all', 'raw', '/repo/src/A.tsx'); + watcher.emit('all', 'error', new Error('boom')); + + expect(handler).not.toHaveBeenCalled(); + }); + + it('normalises chokidar paths via pathe.normalize before forwarding', () => { + const { server, watcher } = createFakeServer(); + const adapter = createViteChangeDetectionAdapter(server); + const handler = vi.fn(); + adapter.onFileChange(handler); + + // Path with `/./` and mixed-case noise that pathe.normalize collapses. + watcher.emit('all', 'change', '/repo/src/./A.tsx'); + + expect(handler).toHaveBeenCalledWith({ + kind: 'change', + path: '/repo/src/A.tsx', + }); + }); + + it('returns an unsubscribe function that removes the listener', () => { + const { server, watcher } = createFakeServer(); + const adapter = createViteChangeDetectionAdapter(server); + const handler = vi.fn(); + const unsubscribe = adapter.onFileChange(handler); + + watcher.emit('all', 'change', '/repo/src/A.tsx'); + expect(handler).toHaveBeenCalledTimes(1); + + unsubscribe(); + watcher.emit('all', 'change', '/repo/src/B.tsx'); + expect(handler).toHaveBeenCalledTimes(1); + }); +}); diff --git a/code/builders/builder-vite/src/change-detection-adapter/index.ts b/code/builders/builder-vite/src/change-detection-adapter/index.ts new file mode 100644 index 000000000000..edbc4812ad10 --- /dev/null +++ b/code/builders/builder-vite/src/change-detection-adapter/index.ts @@ -0,0 +1,53 @@ +import type { + ChangeDetectionAdapter, + FileChangeEvent, + ModuleResolveConfig, +} from 'storybook/internal/core-server'; + +import { normalize } from 'pathe'; +import type { ViteDevServer } from 'vite'; + +/** + * Vite implementation of {@link ChangeDetectionAdapter}. + * + * - `getResolveConfig()` snapshots `server.config.resolve.alias`, `server.config.resolve.conditions` + * and `server.config.root` once at startup. The detector caches the result. + * - `onFileChange()` subscribes to `server.watcher` (chokidar) and forwards `add`/`change`/`unlink` + * events with normalised absolute paths. Other chokidar event names (`addDir`, `unlinkDir`, + * `ready`, `raw`, `error`) are intentionally filtered out. + */ +export function createViteChangeDetectionAdapter(server: ViteDevServer): ChangeDetectionAdapter { + return { + async getResolveConfig(): Promise { + const resolveOpts = server.config.resolve; + // Vite normalises `resolve.alias` to its array form (`Array<{find, replacement, ...}>`) + // before we ever see it. The detector accepts both Record and Array shapes, so we pass + // the array through unchanged. + const alias = resolveOpts?.alias as ModuleResolveConfig['alias']; + const conditions = resolveOpts?.conditions; + + return { + projectRoot: server.config.root, + alias, + conditions, + }; + }, + + onFileChange(handler) { + const FORWARDED_EVENTS = new Set(['add', 'change', 'unlink']); + const isForwardedEvent = (name: string): name is FileChangeEvent['kind'] => + FORWARDED_EVENTS.has(name as FileChangeEvent['kind']); + + const onAll = (eventName: string, path: string) => { + if (!isForwardedEvent(eventName)) { + return; + } + handler({ kind: eventName, path: normalize(path) }); + }; + server.watcher.on('all', onAll); + return () => { + server.watcher.off('all', onAll); + }; + }, + }; +} diff --git a/code/builders/builder-vite/src/index.test.ts b/code/builders/builder-vite/src/index.test.ts deleted file mode 100644 index 714ce9dddece..000000000000 --- a/code/builders/builder-vite/src/index.test.ts +++ /dev/null @@ -1,414 +0,0 @@ -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; - -import { logger } from 'storybook/internal/node-logger'; -import type { ModuleNode as StorybookModuleNode, Options } from 'storybook/internal/types'; -import type { ViteDevServer } from 'vite'; - -import { bail, onModuleGraphChange, start } from './index.ts'; -import { createViteServer } from './vite-server.ts'; - -vi.mock('storybook/internal/node-logger', { spy: true }); -vi.mock('./vite-server', { spy: true }); - -type ViteModuleNodeLike = { - file: string | null; - type: StorybookModuleNode['type']; - importers: Set; - importedModules: Set; -}; - -function createViteModuleNode( - file: string | null, - type: StorybookModuleNode['type'] = 'js' -): ViteModuleNodeLike { - return { - file, - type, - importers: new Set(), - importedModules: new Set(), - }; -} - -function createFileToModulesMap(...entries: Array<[string, Set]>) { - return new Map(entries) as ViteDevServer['moduleGraph']['fileToModulesMap']; -} - -type WatcherHandler = (...args: unknown[]) => void; -type FakeWatcher = { - on: ReturnType; - off: ReturnType; - emit: (event: string, ...args: unknown[]) => void; - listenerCount: (event: string) => number; -}; - -function createFakeViteServer() { - const watcherListeners = new Map>(); - const watcher = {} as FakeWatcher; - - watcher.on = vi.fn((event: string, handler: WatcherHandler) => { - const handlers = watcherListeners.get(event) ?? new Set(); - handlers.add(handler); - watcherListeners.set(event, handlers); - return watcher; - }); - watcher.off = vi.fn((event: string, handler: WatcherHandler) => { - watcherListeners.get(event)?.delete(handler); - return watcher; - }); - watcher.emit = (event: string, ...args: unknown[]) => { - watcherListeners.get(event)?.forEach((handler) => { - handler(...args); - }); - watcherListeners.get('all')?.forEach((handler) => { - handler(event, ...args); - }); - }; - watcher.listenerCount = (event: string) => watcherListeners.get(event)?.size ?? 0; - - return { - watcher, - moduleGraph: { - fileToModulesMap: createFileToModulesMap(), - }, - middlewares: { - handle: vi.fn(), - }, - warmupRequest: vi.fn().mockResolvedValue(undefined), - transformIndexHtml: vi.fn().mockResolvedValue(''), - waitForRequestsIdle: vi.fn().mockResolvedValue(undefined), - close: vi.fn().mockResolvedValue(undefined), - } as unknown as ViteDevServer & { - watcher: typeof watcher; - }; -} - -function createStartArgs(storyImportPaths: string[] = []): Parameters[0] { - const indexGenerator = { - getIndex: vi.fn().mockResolvedValue({ - entries: Object.fromEntries( - storyImportPaths.map((importPath, index) => [`story-${index}`, { importPath }]) - ), - }), - }; - - return { - startTime: process.hrtime(), - options: { - presets: { - apply: vi.fn().mockResolvedValue(indexGenerator), - }, - } as unknown as Options, - router: { - get: vi.fn(), - use: vi.fn(), - } as unknown as Parameters[0]['router'], - server: {} as Parameters[0]['server'], - channel: {} as Parameters[0]['channel'], - }; -} - -describe('onModuleGraphChange', () => { - let fakeViteServer: ReturnType; - - async function startChangeDetection( - fileToModulesMap = createFileToModulesMap([ - '/src/Button.tsx', - new Set([createViteModuleNode('/src/Button.tsx')]), - ]) - ) { - fakeViteServer.moduleGraph.fileToModulesMap = fileToModulesMap; - await start(createStartArgs([...fileToModulesMap.keys()])); - await vi.advanceTimersByTimeAsync(1000); - } - - beforeEach(() => { - vi.useFakeTimers(); - fakeViteServer = createFakeViteServer(); - vi.mocked(createViteServer).mockResolvedValue(fakeViteServer); - vi.mocked(logger.error).mockImplementation(() => undefined); - }); - - afterEach(async () => { - await bail(); - vi.useRealTimers(); - vi.clearAllMocks(); - }); - - it('registers callbacks and unsubscribes them', async () => { - const cb = vi.fn(); - const unsubscribe = onModuleGraphChange(cb); - - expect(unsubscribe).toEqual(expect.any(Function)); - - await startChangeDetection(); - cb.mockClear(); - - fakeViteServer.watcher.emit('change', '/src/Button.tsx'); - - await vi.advanceTimersByTimeAsync(100); - - expect(cb).toHaveBeenCalledTimes(1); - - unsubscribe(); - - fakeViteServer.watcher.emit('change', '/src/Button.tsx'); - await vi.advanceTimersByTimeAsync(100); - - expect(cb).toHaveBeenCalledTimes(1); - }); - - it('passes the module graph payload to listeners', async () => { - const entry = createViteModuleNode('/src/Button.tsx'); - const fileToModulesMap = createFileToModulesMap(['/src/Button.tsx', new Set([entry])]); - - const cb = vi.fn(); - onModuleGraphChange(cb); - - await startChangeDetection(fileToModulesMap); - cb.mockClear(); - - fakeViteServer.watcher.emit('change', '/src/Button.tsx'); - - await vi.advanceTimersByTimeAsync(100); - - const event = cb.mock.calls[0]?.[0]; - const moduleGraph = event?.moduleGraph; - - expect(cb).toHaveBeenCalledWith({ - type: 'moduleGraph', - moduleGraph: expect.any(Map), - }); - expect(moduleGraph?.has('/src/Button.tsx')).toBe(true); - }); - - it('triggers change events after the debounce delay', async () => { - const cb = vi.fn(); - onModuleGraphChange(cb); - - await startChangeDetection(); - cb.mockClear(); - - fakeViteServer.watcher.emit('change', '/src/Button.tsx'); - - await vi.advanceTimersByTimeAsync(50); - expect(cb).not.toHaveBeenCalled(); - - await vi.advanceTimersByTimeAsync(50); - expect(cb).toHaveBeenCalledTimes(1); - }); - - it('triggers add events after the debounce delay', async () => { - const cb = vi.fn(); - onModuleGraphChange(cb); - - await startChangeDetection(); - cb.mockClear(); - - fakeViteServer.watcher.emit('add', '/src/Button.tsx'); - - await vi.advanceTimersByTimeAsync(100); - - expect(cb).toHaveBeenCalledTimes(1); - }); - - it('triggers unlink events after the debounce delay', async () => { - const cb = vi.fn(); - onModuleGraphChange(cb); - - await startChangeDetection(); - cb.mockClear(); - - fakeViteServer.watcher.emit('unlink', '/src/Button.tsx'); - - await vi.advanceTimersByTimeAsync(100); - - expect(cb).toHaveBeenCalledTimes(1); - }); - - it('debounces multiple rapid events into a single callback', async () => { - const cb = vi.fn(); - onModuleGraphChange(cb); - - await startChangeDetection(); - cb.mockClear(); - - fakeViteServer.watcher.emit('change', '/src/Button.tsx'); - fakeViteServer.watcher.emit('add', '/src/Button.tsx'); - fakeViteServer.watcher.emit('change', '/src/Button.tsx'); - - await vi.advanceTimersByTimeAsync(100); - - expect(cb).toHaveBeenCalledTimes(1); - }); - - it('notifies multiple listeners', async () => { - const cb1 = vi.fn(); - const cb2 = vi.fn(); - - onModuleGraphChange(cb1); - onModuleGraphChange(cb2); - - await startChangeDetection(); - cb1.mockClear(); - cb2.mockClear(); - - fakeViteServer.watcher.emit('change', '/src/Button.tsx'); - - await vi.advanceTimersByTimeAsync(100); - - expect(cb1).toHaveBeenCalledTimes(1); - expect(cb2).toHaveBeenCalledTimes(1); - }); - - it('removes the all-event watcher during bail to avoid leaks across restarts', async () => { - const cb = vi.fn(); - onModuleGraphChange(cb); - - await startChangeDetection(); - expect(fakeViteServer.watcher.listenerCount('all')).toBe(1); - - await bail(); - expect(fakeViteServer.watcher.listenerCount('all')).toBe(0); - - await start(createStartArgs(['/src/Button.tsx'])); - await vi.advanceTimersByTimeAsync(1000); - expect(fakeViteServer.watcher.listenerCount('all')).toBe(0); - - fakeViteServer.watcher.emit('change', '/src/Button.tsx'); - await vi.advanceTimersByTimeAsync(100); - - expect(cb).not.toHaveBeenCalled(); - expect(fakeViteServer.watcher.off).toHaveBeenCalledWith('all', expect.any(Function)); - }); - - it('clears the module-graph polling interval during bail', async () => { - onModuleGraphChange(vi.fn()); - - await start(createStartArgs()); - await vi.advanceTimersByTimeAsync(0); - - expect(vi.getTimerCount()).toBe(1); - - await bail(); - - fakeViteServer.moduleGraph.fileToModulesMap = createFileToModulesMap([ - '/src/Button.tsx', - new Set([createViteModuleNode('/src/Button.tsx')]), - ]); - - await vi.advanceTimersByTimeAsync(1000); - - expect(vi.getTimerCount()).toBe(0); - expect(fakeViteServer.waitForRequestsIdle).not.toHaveBeenCalled(); - }); - - it('allows listeners registered after start and runs change detection', async () => { - await start(createStartArgs(['/src/Button.tsx'])); - await vi.advanceTimersByTimeAsync(1000); - - fakeViteServer.moduleGraph.fileToModulesMap = createFileToModulesMap([ - '/src/Button.tsx', - new Set([createViteModuleNode('/src/Button.tsx')]), - ]); - - const cb = vi.fn(); - onModuleGraphChange(cb); - await vi.advanceTimersByTimeAsync(1000); - - cb.mockClear(); - fakeViteServer.watcher.emit('change', '/src/Button.tsx'); - await vi.advanceTimersByTimeAsync(100); - - expect(cb).toHaveBeenCalledTimes(1); - }); - - it('does not reattach the watcher if bail runs while waiting for idle requests', async () => { - const cb = vi.fn(); - onModuleGraphChange(cb); - - let resolveIdle: (() => void) | undefined; - fakeViteServer.moduleGraph.fileToModulesMap = createFileToModulesMap([ - '/src/Button.tsx', - new Set([createViteModuleNode('/src/Button.tsx')]), - ]); - fakeViteServer.waitForRequestsIdle = vi.fn().mockImplementation( - () => - new Promise((resolve) => { - resolveIdle = resolve; - }) - ); - - await start(createStartArgs(['/src/Button.tsx'])); - await vi.advanceTimersByTimeAsync(1000); - - await bail(); - resolveIdle?.(); - await Promise.resolve(); - await Promise.resolve(); - - expect(fakeViteServer.watcher.listenerCount('all')).toBe(0); - expect(fakeViteServer.watcher.on).not.toHaveBeenCalledWith('all', expect.any(Function)); - expect(cb).not.toHaveBeenCalled(); - }); - - it('logs and swallows rejected startup work', async () => { - const cb = vi.fn(); - onModuleGraphChange(cb); - - const args = createStartArgs(); - vi.mocked(args.options.presets.apply).mockResolvedValue({ - getIndex: vi.fn().mockRejectedValue(new Error('index failed')), - } as never); - - await expect(start(args)).resolves.toBeDefined(); - await Promise.resolve(); - - expect(logger.error).toHaveBeenCalledWith('Failed to initialize Vite change detection'); - expect(logger.error).toHaveBeenCalledWith(expect.objectContaining({ message: 'index failed' })); - expect(cb).toHaveBeenCalledWith({ - type: 'error', - error: expect.objectContaining({ message: 'index failed' }), - }); - expect(vi.getTimerCount()).toBe(0); - }); - - it('logs polling failures and clears the interval', async () => { - const cb = vi.fn(); - onModuleGraphChange(cb); - fakeViteServer.moduleGraph.fileToModulesMap = createFileToModulesMap([ - '/src/Button.tsx', - new Set([createViteModuleNode('/src/Button.tsx')]), - ]); - fakeViteServer.waitForRequestsIdle = vi.fn().mockRejectedValue(new Error('idle failed')); - - await start(createStartArgs(['/src/Button.tsx'])); - await vi.advanceTimersByTimeAsync(1000); - - expect(logger.error).toHaveBeenCalledWith('Failed to complete Vite change detection startup'); - expect(logger.error).toHaveBeenCalledWith(expect.objectContaining({ message: 'idle failed' })); - expect(cb).toHaveBeenCalledWith({ - type: 'error', - error: expect.objectContaining({ message: 'idle failed' }), - }); - expect(vi.getTimerCount()).toBe(0); - expect(fakeViteServer.watcher.listenerCount('all')).toBe(0); - }); - - it('notifies listeners when module graph startup times out', async () => { - const cb = vi.fn(); - onModuleGraphChange(cb); - - await start(createStartArgs(['/src/Button.tsx'])); - await vi.advanceTimersByTimeAsync(31_000); - - expect(logger.error).toHaveBeenCalledWith('Failed to complete Vite change detection startup'); - expect(cb).toHaveBeenCalledWith({ - type: 'unavailable', - reason: 'Timed out while waiting for the Vite module graph to initialize', - error: expect.objectContaining({ - message: 'Timed out while waiting for the Vite module graph to initialize', - }), - }); - expect(vi.getTimerCount()).toBe(0); - }); -}); diff --git a/code/builders/builder-vite/src/index.ts b/code/builders/builder-vite/src/index.ts index 13634fd352da..028c344674c0 100644 --- a/code/builders/builder-vite/src/index.ts +++ b/code/builders/builder-vite/src/index.ts @@ -2,23 +2,15 @@ import { readFile } from 'node:fs/promises'; import { fileURLToPath } from 'node:url'; -import { logger } from 'storybook/internal/node-logger'; import { NoStatsForViteDevError } from 'storybook/internal/server-errors'; -import type { StoryIndexGenerator } from 'storybook/internal/core-server'; -import type { - Builder, - Middleware, - ModuleGraph, - ModuleGraphChangeEvent, - Options, -} from 'storybook/internal/types'; +import type { Builder, Middleware, Options } from 'storybook/internal/types'; import type { ViteDevServer } from 'vite'; import { build as viteBuild } from './build.ts'; +import { createViteChangeDetectionAdapter } from './change-detection-adapter/index.ts'; import type { ViteBuilder } from './types.ts'; import { createViteServer } from './vite-server.ts'; -import { buildModuleGraph } from './utils/build-module-graph.ts'; export { withoutVitePlugins } from './utils/without-vite-plugins.ts'; export { hasVitePlugins } from './utils/has-vite-plugins.ts'; @@ -42,142 +34,25 @@ function iframeHandler(options: Options, server: ViteDevServer): Middleware { } let server: ViteDevServer; -let lastBuilderOptions: Options | undefined; -const listeners = new Set<(event: ModuleGraphChangeEvent) => void>(); -let debounce: ReturnType | undefined; -let watcherChangeHandler: (() => void) | undefined; -let waitForModuleGraph: ReturnType | undefined; -let moduleGraphTrackingStarted = false; - -function clearModuleGraphPolling(): void { - if (waitForModuleGraph) { - clearInterval(waitForModuleGraph); - waitForModuleGraph = undefined; - } -} - -function notifyListeners(moduleGraph: ModuleGraph): void { - listeners.forEach((listener) => { - listener({ type: 'moduleGraph', moduleGraph }); - }); -} - -function notifyListenersOfStartupFailure( - event: Extract -): void { - listeners.forEach((listener) => { - listener(event); - }); -} export async function bail(): Promise { - if (watcherChangeHandler) { - server?.watcher.off('all', watcherChangeHandler); - watcherChangeHandler = undefined; - } - - clearModuleGraphPolling(); - - if (debounce) { - clearTimeout(debounce); - debounce = undefined; - } - - moduleGraphTrackingStarted = false; - lastBuilderOptions = undefined; - listeners.clear(); return server?.close(); } -function startModuleGraphTracking(): void { - if (moduleGraphTrackingStarted || listeners.size === 0 || !server || !lastBuilderOptions) { - return; +/** + * Returns a {@link ChangeDetectionAdapter} bound to the Vite dev server created by `start()`. + * + * Throws if called before `start()` has resolved (i.e. before the Vite dev server exists). + */ +export const changeDetectionAdapter: NonNullable< + Builder['changeDetectionAdapter'] +> = () => { + if (!server) { + throw new Error( + 'builder-vite: changeDetectionAdapter() called before start(); the Vite dev server is not ready yet.' + ); } - - moduleGraphTrackingStarted = true; - - // Debounce handler to prevent multiple callback invocations when multiple files are edited - watcherChangeHandler = () => { - clearTimeout(debounce); - debounce = setTimeout(() => { - notifyListeners(buildModuleGraph(server.moduleGraph.fileToModulesMap)); - }, 100); - }; - - startChangeDetection(lastBuilderOptions).catch((error) => { - clearModuleGraphPolling(); - logger.error('Failed to initialize Vite change detection'); - logger.error(error instanceof Error ? error : String(error)); - notifyListenersOfStartupFailure({ - type: 'error', - error: error instanceof Error ? error : new Error(String(error)), - }); - }); -} - -export const onModuleGraphChange: NonNullable['onModuleGraphChange']> = (cb) => { - listeners.add(cb); - startModuleGraphTracking(); - - return () => { - listeners.delete(cb); - }; -}; - -const startChangeDetection = async (options: Options) => { - const startTime = process.hrtime(); - const indexGenerator = await options.presets.apply('storyIndexGenerator'); - const storyIndex = await indexGenerator.getIndex(); - const importPaths = new Set(Object.values(storyIndex.entries).map((entry) => entry.importPath)); - - // Warm up the module graph for all story files - await Promise.all(Array.from(importPaths, (importPath) => server.warmupRequest(importPath))); - - // Wait for the module graph to be ready by polling for it to be non-empty - waitForModuleGraph = setInterval(() => { - void (async () => { - try { - if (!watcherChangeHandler) { - clearModuleGraphPolling(); - return; - } - - if (process.hrtime(startTime)[0] > 30) { - clearModuleGraphPolling(); - const error = new Error( - 'Timed out while waiting for the Vite module graph to initialize' - ); - logger.error('Failed to complete Vite change detection startup'); - logger.error(error); - notifyListenersOfStartupFailure({ - type: 'unavailable', - reason: error.message, - error, - }); - return; - } - - if (server.moduleGraph.fileToModulesMap.size > 0) { - clearModuleGraphPolling(); - await server.waitForRequestsIdle(); - if (!watcherChangeHandler) { - return; - } - - server.watcher.on('all', watcherChangeHandler); - watcherChangeHandler(); - } - } catch (error) { - clearModuleGraphPolling(); - logger.error('Failed to complete Vite change detection startup'); - logger.error(error instanceof Error ? error : String(error)); - notifyListenersOfStartupFailure({ - type: 'error', - error: error instanceof Error ? error : new Error(String(error)), - }); - } - })(); - }, 1000); + return createViteChangeDetectionAdapter(server); }; export const start: ViteBuilder['start'] = async ({ @@ -186,14 +61,11 @@ export const start: ViteBuilder['start'] = async ({ router, server: devServer, }) => { - lastBuilderOptions = options as Options; server = await createViteServer(options as Options, devServer); router.get('/iframe.html', iframeHandler(options as Options, server)); router.use(server.middlewares); - startModuleGraphTracking(); - return { bail, stats: { diff --git a/code/builders/builder-vite/src/utils/build-module-graph.test.ts b/code/builders/builder-vite/src/utils/build-module-graph.test.ts deleted file mode 100644 index 9545b7b434e8..000000000000 --- a/code/builders/builder-vite/src/utils/build-module-graph.test.ts +++ /dev/null @@ -1,153 +0,0 @@ -import { describe, expect, it, vi } from 'vitest'; - -import type { ModuleNode as StorybookModuleNode } from 'storybook/internal/types'; -import type { ViteDevServer } from 'vite'; - -import { buildModuleGraph } from './build-module-graph.ts'; - -vi.mock('./vite-server', () => ({ - createViteServer: vi.fn(), -})); - -type ViteModuleNodeLike = { - file: string | null; - type: StorybookModuleNode['type']; - importers: Set; - importedModules: Set; -}; - -function createViteModuleNode( - file: string | null, - type: StorybookModuleNode['type'] = 'js' -): ViteModuleNodeLike { - return { - file, - type, - importers: new Set(), - importedModules: new Set(), - }; -} - -function createFileToModulesMap(...entries: Array<[string, Set]>) { - return new Map(entries) as ViteDevServer['moduleGraph']['fileToModulesMap']; -} - -function getFirstNode( - file: string, - moduleGraph: ReturnType -): StorybookModuleNode { - const moduleNode = moduleGraph.get(file)?.values().next().value; - if (!moduleNode) { - throw new Error(`Expected module node for ${file}`); - } - return moduleNode; -} - -describe('buildModuleGraph', () => { - it('converts vite module nodes into the shared module graph shape', () => { - const entry = createViteModuleNode('/src/entry.ts'); - const component = createViteModuleNode('/src/component.ts'); - const styles = createViteModuleNode('/src/component.css', 'css'); - - entry.importedModules.add(component); - component.importers.add(entry); - component.importedModules.add(styles); - styles.importers.add(component); - - const moduleGraph = buildModuleGraph( - createFileToModulesMap( - ['/src/entry.ts', new Set([entry])], - ['/src/component.ts', new Set([component])], - ['/src/component.css', new Set([styles])] - ) - ); - - const entryNode = getFirstNode('/src/entry.ts', moduleGraph); - const componentNode = getFirstNode('/src/component.ts', moduleGraph); - const styleNode = getFirstNode('/src/component.css', moduleGraph); - - expect(entryNode.file).toBe('/src/entry.ts'); - expect(componentNode.type).toBe('js'); - expect(styleNode.type).toBe('css'); - - expect(entryNode.importedModules).toEqual(new Set([componentNode])); - expect(componentNode.importers).toEqual(new Set([entryNode])); - expect(componentNode.importedModules).toEqual(new Set([styleNode])); - expect(styleNode.importers).toEqual(new Set([componentNode])); - }); - - it('reuses the same converted node identity across relationships', () => { - const shared = createViteModuleNode('/src/shared.ts'); - const importerA = createViteModuleNode('/src/a.ts'); - const importerB = createViteModuleNode('/src/b.ts'); - - importerA.importedModules.add(shared); - importerB.importedModules.add(shared); - shared.importers.add(importerA); - shared.importers.add(importerB); - - const moduleGraph = buildModuleGraph( - createFileToModulesMap( - ['/src/shared.ts', new Set([shared])], - ['/src/a.ts', new Set([importerA])], - ['/src/b.ts', new Set([importerB])] - ) - ); - - const sharedNode = getFirstNode('/src/shared.ts', moduleGraph); - const importerANode = getFirstNode('/src/a.ts', moduleGraph); - const importerBNode = getFirstNode('/src/b.ts', moduleGraph); - - expect(importerANode.importedModules.has(sharedNode)).toBe(true); - expect(importerBNode.importedModules.has(sharedNode)).toBe(true); - expect(sharedNode.importers).toEqual(new Set([importerANode, importerBNode])); - }); - - it('skips related vite module nodes without a file', () => { - const entry = createViteModuleNode('/src/entry.ts'); - const virtualModule = createViteModuleNode(null); - - entry.importedModules.add(virtualModule); - virtualModule.importers.add(entry); - - const moduleGraph = buildModuleGraph( - createFileToModulesMap(['/src/entry.ts', new Set([entry])]) - ); - const entryNode = getFirstNode('/src/entry.ts', moduleGraph); - - expect(moduleGraph.size).toBe(1); - expect(entryNode.importedModules.size).toBe(0); - }); - - it('preserves edges for related vite module nodes discovered before their file path is known', () => { - const entry = createViteModuleNode('/src/entry.ts'); - const component = createViteModuleNode(null); - - entry.importedModules.add(component); - component.importers.add(entry); - - const moduleGraph = buildModuleGraph( - createFileToModulesMap( - ['/src/entry.ts', new Set([entry])], - ['/src/component.ts', new Set([component])] - ) - ); - - const entryNode = getFirstNode('/src/entry.ts', moduleGraph); - const componentNode = getFirstNode('/src/component.ts', moduleGraph); - - expect(entryNode.importedModules).toEqual(new Set([componentNode])); - expect(componentNode.importers).toEqual(new Set([entryNode])); - }); - - it('keeps multiple module identities for the same file', () => { - const clientModule = createViteModuleNode('/src/shared.ts'); - const ssrModule = createViteModuleNode('/src/shared.ts'); - - const moduleGraph = buildModuleGraph( - createFileToModulesMap(['/src/shared.ts', new Set([clientModule, ssrModule])]) - ); - - expect(moduleGraph.get('/src/shared.ts')?.size).toBe(2); - }); -}); diff --git a/code/builders/builder-vite/src/utils/build-module-graph.ts b/code/builders/builder-vite/src/utils/build-module-graph.ts deleted file mode 100644 index 4d14235bd88e..000000000000 --- a/code/builders/builder-vite/src/utils/build-module-graph.ts +++ /dev/null @@ -1,82 +0,0 @@ -import type { ModuleGraph, ModuleNode } from 'storybook/internal/types'; - -import type { ViteDevServer, ModuleNode as ViteModuleNode } from 'vite'; - -export function buildModuleGraph( - fileToModulesMap: ViteDevServer['moduleGraph']['fileToModulesMap'] -): ModuleGraph { - const moduleGraph: ModuleGraph = new Map(); - const moduleNodeMap = new WeakMap(); - const getModuleFileFromMap = (viteModuleNode: { - file: string | null; - type: ViteModuleNode['type']; - importers: Set; - importedModules: Set; - }): string | undefined => { - for (const [filePath, viteModuleSet] of fileToModulesMap.entries()) { - if (viteModuleSet.has(viteModuleNode as ViteModuleNode)) { - return filePath; - } - } - }; - - const getOrCreateModuleNode = ( - viteModuleNode: { - file: string | null; - type: ViteModuleNode['type']; - importers: Set; - importedModules: Set; - }, - fallbackFile?: string - ): ModuleNode | undefined => { - const file = viteModuleNode.file ?? fallbackFile; - if (!file) { - return undefined; - } - - const existingNode = moduleNodeMap.get(viteModuleNode); - if (existingNode) { - return existingNode; - } - - const moduleNode: ModuleNode = { - file, - type: viteModuleNode.type, - importers: new Set(), - importedModules: new Set(), - }; - moduleNodeMap.set(viteModuleNode, moduleNode); - - const moduleSet = moduleGraph.get(file) ?? new Set(); - moduleSet.add(moduleNode); - moduleGraph.set(file, moduleSet); - - return moduleNode; - }; - - fileToModulesMap.forEach((viteModuleSet, filePath) => { - viteModuleSet.forEach((viteModuleNode) => { - const moduleNode = getOrCreateModuleNode(viteModuleNode, filePath); - if (moduleNode) { - viteModuleNode.importers.forEach((importer) => { - const importerNode = - getOrCreateModuleNode(importer) ?? - getOrCreateModuleNode(importer, getModuleFileFromMap(importer)); - if (importerNode) { - moduleNode.importers.add(importerNode); - } - }); - viteModuleNode.importedModules.forEach((importedModule) => { - const importedModuleNode = - getOrCreateModuleNode(importedModule) ?? - getOrCreateModuleNode(importedModule, getModuleFileFromMap(importedModule)); - if (importedModuleNode) { - moduleNode.importedModules.add(importedModuleNode); - } - }); - } - }); - }); - - return moduleGraph; -} diff --git a/code/builders/builder-webpack5/src/change-detection-adapter/index.test.ts b/code/builders/builder-webpack5/src/change-detection-adapter/index.test.ts new file mode 100644 index 000000000000..d20e2330d026 --- /dev/null +++ b/code/builders/builder-webpack5/src/change-detection-adapter/index.test.ts @@ -0,0 +1,215 @@ +// Tests the webpack implementation of ChangeDetectionAdapter — resolve-config extraction +// and watchRun-based file-change event normalisation. +import { describe, expect, it, vi } from 'vitest'; + +import { createWebpackChangeDetectionAdapter } from './index.ts'; + +interface FakeTapable { + tap(pluginName: string, fn: (compiler: FakeCompiler) => void): void; +} + +interface FakeCompiler { + context: string; + options: { + resolve: { + alias?: unknown; + conditionNames?: string[]; + }; + }; + hooks: { + watchRun: FakeTapable; + }; + modifiedFiles?: ReadonlySet; + removedFiles?: ReadonlySet; +} + +function createFakeCompiler(overrides: Partial = {}): { + compiler: FakeCompiler; + triggerWatchRun: (modifiedFiles?: string[], removedFiles?: string[]) => void; +} { + let watchRunCallback: ((c: FakeCompiler) => void) | undefined; + + const compiler: FakeCompiler = { + context: '/repo', + options: { + resolve: {}, + }, + hooks: { + watchRun: { + tap(_pluginName, fn) { + watchRunCallback = fn; + }, + }, + }, + ...overrides, + }; + + function triggerWatchRun(modifiedFiles: string[] = [], removedFiles: string[] = []): void { + const ctx: FakeCompiler = { + ...compiler, + modifiedFiles: new Set(modifiedFiles), + removedFiles: new Set(removedFiles), + }; + watchRunCallback?.(ctx); + } + + return { compiler, triggerWatchRun }; +} + +describe('createWebpackChangeDetectionAdapter', () => { + describe('getResolveConfig()', () => { + it('returns projectRoot from compiler.context', async () => { + const { compiler } = createFakeCompiler({ context: '/my/project' }); + const adapter = createWebpackChangeDetectionAdapter(compiler as any); + const config = await adapter.getResolveConfig(); + expect(config.projectRoot).toBe('/my/project'); + }); + + it('returns conditions from resolve.conditionNames', async () => { + const { compiler } = createFakeCompiler({ + options: { resolve: { conditionNames: ['import', 'module'] } }, + }); + const adapter = createWebpackChangeDetectionAdapter(compiler as any); + const config = await adapter.getResolveConfig(); + expect(config.conditions).toEqual(['import', 'module']); + }); + + it('normalises object-form alias to Record', async () => { + const { compiler } = createFakeCompiler({ + options: { + resolve: { + alias: { + '@': '/repo/src', + utils: '/repo/utils', + disabled: false, + multi: ['/repo/multi-a', '/repo/multi-b'], + }, + }, + }, + }); + const adapter = createWebpackChangeDetectionAdapter(compiler as any); + const config = await adapter.getResolveConfig(); + expect(config.alias).toEqual({ + '@': '/repo/src', + utils: '/repo/utils', + multi: '/repo/multi-a', + // `disabled: false` is skipped + }); + }); + + it('normalises array-form alias to Array<{ find, replacement }>', async () => { + const { compiler } = createFakeCompiler({ + options: { + resolve: { + alias: [ + { name: '@', alias: '/repo/src' }, + { name: 'utils', alias: ['/repo/utils-a'] }, + { name: 'gone', alias: false }, + ], + }, + }, + }); + const adapter = createWebpackChangeDetectionAdapter(compiler as any); + const config = await adapter.getResolveConfig(); + expect(config.alias).toEqual([ + { find: '@', replacement: '/repo/src' }, + { find: 'utils', replacement: '/repo/utils-a' }, + // `gone: false` is skipped + ]); + }); + + it('returns undefined alias when resolve has no alias', async () => { + const { compiler } = createFakeCompiler({ options: { resolve: {} } }); + const adapter = createWebpackChangeDetectionAdapter(compiler as any); + const config = await adapter.getResolveConfig(); + expect(config.alias).toBeUndefined(); + }); + }); + + describe('onFileChange()', () => { + it('emits kind:"add" for first-seen paths in modifiedFiles', () => { + const { compiler, triggerWatchRun } = createFakeCompiler(); + const adapter = createWebpackChangeDetectionAdapter(compiler as any); + const handler = vi.fn(); + adapter.onFileChange(handler); + + triggerWatchRun(['/repo/src/A.tsx']); + + expect(handler).toHaveBeenCalledWith({ kind: 'add', path: '/repo/src/A.tsx' }); + }); + + it('emits kind:"change" for paths seen in a previous watchRun', () => { + const { compiler, triggerWatchRun } = createFakeCompiler(); + const adapter = createWebpackChangeDetectionAdapter(compiler as any); + const handler = vi.fn(); + adapter.onFileChange(handler); + + triggerWatchRun(['/repo/src/A.tsx']); // first time → add + triggerWatchRun(['/repo/src/A.tsx']); // second time → change + + expect(handler).toHaveBeenNthCalledWith(1, { kind: 'add', path: '/repo/src/A.tsx' }); + expect(handler).toHaveBeenNthCalledWith(2, { kind: 'change', path: '/repo/src/A.tsx' }); + }); + + it('emits kind:"unlink" for removedFiles and forgets the path', () => { + const { compiler, triggerWatchRun } = createFakeCompiler(); + const adapter = createWebpackChangeDetectionAdapter(compiler as any); + const handler = vi.fn(); + adapter.onFileChange(handler); + + triggerWatchRun(['/repo/src/A.tsx']); // add + triggerWatchRun([], ['/repo/src/A.tsx']); // unlink + + expect(handler).toHaveBeenNthCalledWith(2, { kind: 'unlink', path: '/repo/src/A.tsx' }); + }); + + it('emits kind:"add" again after a path was unlinked and re-added', () => { + const { compiler, triggerWatchRun } = createFakeCompiler(); + const adapter = createWebpackChangeDetectionAdapter(compiler as any); + const handler = vi.fn(); + adapter.onFileChange(handler); + + triggerWatchRun(['/repo/src/A.tsx']); // add + triggerWatchRun([], ['/repo/src/A.tsx']); // unlink + triggerWatchRun(['/repo/src/A.tsx']); // add again + + expect(handler).toHaveBeenNthCalledWith(3, { kind: 'add', path: '/repo/src/A.tsx' }); + }); + + it('normalises paths via pathe.normalize before forwarding', () => { + const { compiler, triggerWatchRun } = createFakeCompiler(); + const adapter = createWebpackChangeDetectionAdapter(compiler as any); + const handler = vi.fn(); + adapter.onFileChange(handler); + + triggerWatchRun(['/repo/src/./A.tsx']); + + expect(handler).toHaveBeenCalledWith({ kind: 'add', path: '/repo/src/A.tsx' }); + }); + + it('does not emit events after the unsubscribe function is called', () => { + const { compiler, triggerWatchRun } = createFakeCompiler(); + const adapter = createWebpackChangeDetectionAdapter(compiler as any); + const handler = vi.fn(); + const unsubscribe = adapter.onFileChange(handler); + + triggerWatchRun(['/repo/src/A.tsx']); + expect(handler).toHaveBeenCalledTimes(1); + + unsubscribe(); + triggerWatchRun(['/repo/src/B.tsx']); + expect(handler).toHaveBeenCalledTimes(1); + }); + + it('emits no events when modifiedFiles and removedFiles are empty', () => { + const { compiler, triggerWatchRun } = createFakeCompiler(); + const adapter = createWebpackChangeDetectionAdapter(compiler as any); + const handler = vi.fn(); + adapter.onFileChange(handler); + + triggerWatchRun([], []); + + expect(handler).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/code/builders/builder-webpack5/src/change-detection-adapter/index.ts b/code/builders/builder-webpack5/src/change-detection-adapter/index.ts new file mode 100644 index 000000000000..244e056d439e --- /dev/null +++ b/code/builders/builder-webpack5/src/change-detection-adapter/index.ts @@ -0,0 +1,95 @@ +import type { + ChangeDetectionAdapter, + FileChangeEvent, + ModuleResolveConfig, +} from 'storybook/internal/core-server'; + +import { normalize } from 'pathe'; +import type { Compiler } from 'webpack'; + +/** + * Webpack implementation of {@link ChangeDetectionAdapter}. + * + * - `getResolveConfig()` reads `compiler.options.resolve` and `compiler.context` once at startup. + * - `onFileChange()` taps `compiler.hooks.watchRun` and forwards `modifiedFiles` as `add`/`change` + * and `removedFiles` as `unlink` events. File kind (add vs change) is inferred by tracking which + * paths webpack has reported at least once. + * + * Note: on first watch run `modifiedFiles` may be undefined (initial full build). Events are only + * emitted for subsequent incremental runs where webpack populates the sets. + */ +export function createWebpackChangeDetectionAdapter(compiler: Compiler): ChangeDetectionAdapter { + return { + async getResolveConfig(): Promise { + const resolveOpts = compiler.options.resolve; + return { + projectRoot: compiler.context, + alias: normaliseWebpackAlias(resolveOpts.alias), + conditions: resolveOpts.conditionNames, + }; + }, + + onFileChange(handler: (event: FileChangeEvent) => void): () => void { + let active = true; + const seenFiles = new Set(); + + compiler.hooks.watchRun.tap('StorybookChangeDetection', (watchingCompiler) => { + if (!active) return; + + for (const filePath of watchingCompiler.modifiedFiles ?? []) { + const path = normalize(filePath); + const kind: FileChangeEvent['kind'] = seenFiles.has(path) ? 'change' : 'add'; + seenFiles.add(path); + handler({ kind, path }); + } + + for (const filePath of watchingCompiler.removedFiles ?? []) { + const path = normalize(filePath); + seenFiles.delete(path); + handler({ kind: 'unlink', path }); + } + }); + + return () => { + active = false; + }; + }, + }; +} + +type WebpackResolveAlias = NonNullable< + NonNullable['alias'] +>; + +function normaliseWebpackAlias( + alias: WebpackResolveAlias | undefined +): ModuleResolveConfig['alias'] | undefined { + if (!alias) return undefined; + + if (Array.isArray(alias)) { + const entries: Array<{ find: string; replacement: string }> = []; + for (const entry of alias) { + const replacement = + typeof entry.alias === 'string' + ? entry.alias + : Array.isArray(entry.alias) && entry.alias.length > 0 + ? entry.alias[0] + : null; + if (replacement != null) { + entries.push({ find: entry.name, replacement }); + } + } + return entries.length > 0 ? entries : undefined; + } + + const record: Record = {}; + for (const [key, value] of Object.entries(alias as Record)) { + if (typeof value === 'string') { + record[key] = value; + } else if (Array.isArray(value) && value.length > 0) { + record[key] = value[0]; + } + // false = disabled alias, skip + } + return Object.keys(record).length > 0 ? record : undefined; +} diff --git a/code/builders/builder-webpack5/src/index.ts b/code/builders/builder-webpack5/src/index.ts index f4312b4c0939..042ffc24ab9a 100644 --- a/code/builders/builder-webpack5/src/index.ts +++ b/code/builders/builder-webpack5/src/index.ts @@ -20,6 +20,8 @@ import webpackModule from 'webpack'; import webpackDevMiddleware from 'webpack-dev-middleware'; import webpackHotMiddleware from 'webpack-hot-middleware'; +import { createWebpackChangeDetectionAdapter } from './change-detection-adapter/index.ts'; + export * from './types.ts'; export * from './preview/virtual-module-mapping.ts'; @@ -38,6 +40,7 @@ const corePath = dirname(fileURLToPath(import.meta.resolve('storybook/package.js let compilation: ReturnType | undefined; let reject: (reason?: any) => void; +let activeCompiler: import('webpack').Compiler | undefined; type WebpackBuilder = Builder; type Unpromise> = T extends Promise ? U : never; @@ -97,7 +100,7 @@ export const bail: WebpackBuilder['bail'] = async () => { reject(); } // we wait for the compiler to finish it's work, so it's command-line output doesn't interfere - return new Promise((res, rej) => { + await new Promise((res, rej) => { if (process && compilation) { try { compilation.close(() => res()); @@ -110,6 +113,21 @@ export const bail: WebpackBuilder['bail'] = async () => { res(); } }); + activeCompiler = undefined; +}; + +/** + * Returns a {@link ChangeDetectionAdapter} bound to the webpack compiler created by `start()`. + * + * Throws if called before `start()` has resolved (i.e. before the compiler exists). + */ +export const changeDetectionAdapter: NonNullable = () => { + if (!activeCompiler) { + throw new Error( + 'builder-webpack5: changeDetectionAdapter() called before start(); the webpack compiler is not ready yet.' + ); + } + return createWebpackChangeDetectionAdapter(activeCompiler); }; /** @@ -143,6 +161,8 @@ const starter: StarterFunction = async function* starterGeneratorFn({ }); } + activeCompiler = compiler; + yield; const modulesCount = await options.cache?.get('modulesCount', 1000); let totalModules: number; diff --git a/code/core/build-config.ts b/code/core/build-config.ts index e12d7c7ceba0..3d09dd7b6020 100644 --- a/code/core/build-config.ts +++ b/code/core/build-config.ts @@ -37,6 +37,14 @@ const config: BuildEntries = { entryPoint: './src/core-server/presets/common-preset.ts', dts: false, }, + { + exportEntries: ['./internal/oxc-parser'], + entryPoint: './src/oxc-parser/index.ts', + }, + { + entryPoint: './src/oxc-parser/worker.ts', + dts: false, + }, { entryPoint: './src/core-server/presets/common-override-preset.ts', exportEntries: ['./internal/core-server/presets/common-override-preset'], diff --git a/code/core/package.json b/code/core/package.json index 8942e5d14aba..9f7ba65dfb16 100644 --- a/code/core/package.json +++ b/code/core/package.json @@ -151,6 +151,11 @@ "code": "./src/node-logger/index.ts", "default": "./dist/node-logger/index.js" }, + "./internal/oxc-parser": { + "types": "./dist/oxc-parser/index.d.ts", + "code": "./src/oxc-parser/index.ts", + "default": "./dist/oxc-parser/index.js" + }, "./internal/preview-api": { "types": "./dist/preview-api/index.d.ts", "code": "./src/preview-api/index.ts", @@ -237,6 +242,8 @@ "@webcontainer/env": "^1.1.1", "esbuild": "^0.18.0 || ^0.19.0 || ^0.20.0 || ^0.21.0 || ^0.22.0 || ^0.23.0 || ^0.24.0 || ^0.25.0 || ^0.26.0 || ^0.27.0", "open": "^10.2.0", + "oxc-parser": "^0.127.0", + "oxc-resolver": "^11.19.1", "recast": "^0.23.5", "semver": "^7.7.3", "use-sync-external-store": "^1.5.0", diff --git a/code/core/src/common/versions.ts b/code/core/src/common/versions.ts index 884ccaaf099d..bffe67fc061a 100644 --- a/code/core/src/common/versions.ts +++ b/code/core/src/common/versions.ts @@ -22,6 +22,7 @@ export default { '@storybook/server-webpack5': '10.4.0-alpha.16', '@storybook/svelte-vite': '10.4.0-alpha.16', '@storybook/sveltekit': '10.4.0-alpha.16', + '@storybook/tanstack-react': '10.4.0-alpha.16', '@storybook/vue3-vite': '10.4.0-alpha.16', '@storybook/web-components-vite': '10.4.0-alpha.16', sb: '10.4.0-alpha.16', @@ -37,7 +38,6 @@ export default { '@storybook/preset-server-webpack': '10.4.0-alpha.16', '@storybook/html': '10.4.0-alpha.16', '@storybook/preact': '10.4.0-alpha.16', - '@storybook/tanstack-react': '10.4.0-alpha.16', '@storybook/react': '10.4.0-alpha.16', '@storybook/server': '10.4.0-alpha.16', '@storybook/svelte': '10.4.0-alpha.16', diff --git a/code/core/src/core-server/change-detection/ChangeDetectionService.test.ts b/code/core/src/core-server/change-detection/ChangeDetectionService.test.ts index 639a4141cb71..23574f05816f 100644 --- a/code/core/src/core-server/change-detection/ChangeDetectionService.test.ts +++ b/code/core/src/core-server/change-detection/ChangeDetectionService.test.ts @@ -1,16 +1,7 @@ -import { join } from 'pathe'; - import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { logger } from 'storybook/internal/node-logger'; -import type { - Builder, - ModuleGraph, - ModuleGraphChangeEvent, - ModuleNode, - Status, - StoryIndex, -} from 'storybook/internal/types'; +import type { Status, StoryIndex } from 'storybook/internal/types'; import { CHANGE_DETECTION_STATUS_TYPE_ID } from 'storybook/internal/types'; import { @@ -18,6 +9,7 @@ import { UNIVERSAL_STATUS_STORE_OPTIONS, } from '../../shared/status-store/index.ts'; import { MockUniversalStore } from '../../shared/universal-store/mock.ts'; +import type { ChangeDetectionAdapter, FileChangeEvent } from './adapters/index.ts'; import { getChangeDetectionReadiness, internal_resetChangeDetectionReadiness } from './index.ts'; import { ChangeDetectionFailureError, ChangeDetectionUnavailableError } from './errors.ts'; import { @@ -26,11 +18,31 @@ import { mergeChangeDetectionStatuses, mergeStatusValues, } from './ChangeDetectionService.ts'; +import * as oxcParser from 'storybook/internal/oxc-parser'; +import { + ChangeDetectionResolverFactory, + DependencyGraphBuilder, + IncrementalPatcher, + ReverseIndexImpl, +} from './dependency-graph/index.ts'; import type { GitDiffResult } from './GitDiffProvider.ts'; import { GitDiffProvider } from './GitDiffProvider.ts'; import type { IndexBaselineService } from './IndexBaselineService.ts'; vi.mock('storybook/internal/node-logger', { spy: true }); +vi.mock('./dependency-graph/index.ts', async (importOriginal) => { + // Keep ReverseIndexImpl + types real so tests can build synthetic indexes; replace the + // ChangeDetectionResolverFactory / DependencyGraphBuilder / IncrementalPatcher constructors + // with `vi.fn()`s so tests can override their behaviour per-case via + // `vi.mocked(Ctor).mockImplementation(...)`. + const actual = await importOriginal(); + return { + ...actual, + ChangeDetectionResolverFactory: vi.fn(), + DependencyGraphBuilder: vi.fn(), + IncrementalPatcher: vi.fn(), + }; +}); function createDeferred() { let resolve!: (value: T) => void; @@ -43,15 +55,6 @@ function createDeferred() { }; } -function createModuleNode(file: string): ModuleNode { - return { - file, - type: 'js', - importers: new Set(), - importedModules: new Set(), - }; -} - function createStoryIndex( entries: Array<{ storyId: string; importPath: string; title?: string; name?: string }> ): StoryIndex { @@ -73,29 +76,50 @@ function createStoryIndex( }; } -function createBuilder() { - let onModuleGraphChange: ((event: ModuleGraphChangeEvent) => void) | undefined; +interface MockAdapterHandle { + adapter: ChangeDetectionAdapter; + emitFileChange: (event: FileChangeEvent) => void; + emitStartupFailure: (event: { reason: string; error?: Error }) => void; + hasFileChangeSubscriber: () => boolean; + hasStartupFailureSubscriber: () => boolean; +} - const builder = { - onModuleGraphChange: vi.fn((callback: (event: ModuleGraphChangeEvent) => void) => { - onModuleGraphChange = callback; - return vi.fn(() => { - onModuleGraphChange = undefined; - }); - }), - } as unknown as Builder; +function createMockAdapter(opts?: { + resolveConfig?: { projectRoot?: string }; + withoutStartupFailure?: boolean; +}): MockAdapterHandle { + const fileHandlers = new Set<(e: FileChangeEvent) => void>(); + const startupHandlers = new Set<(e: { reason: string; error?: Error }) => void>(); + + const adapter: ChangeDetectionAdapter = { + async getResolveConfig() { + return { + projectRoot: opts?.resolveConfig?.projectRoot ?? '/repo', + }; + }, + onFileChange(handler) { + fileHandlers.add(handler); + return () => fileHandlers.delete(handler); + }, + }; + + if (!opts?.withoutStartupFailure) { + adapter.onStartupFailure = (handler) => { + startupHandlers.add(handler); + return () => startupHandlers.delete(handler); + }; + } return { - builder, - emit(moduleGraph: ModuleGraph) { - onModuleGraphChange?.({ type: 'moduleGraph', moduleGraph }); + adapter, + emitFileChange: (event) => { + fileHandlers.forEach((h) => h(event)); }, - emitUnavailable(reason: string, error?: Error) { - onModuleGraphChange?.({ type: 'unavailable', reason, error }); - }, - emitError(error: Error) { - onModuleGraphChange?.({ type: 'error', error }); + emitStartupFailure: (event) => { + startupHandlers.forEach((h) => h(event)); }, + hasFileChangeSubscriber: () => fileHandlers.size > 0, + hasStartupFailureSubscriber: () => startupHandlers.size > 0, }; } @@ -114,6 +138,7 @@ class MockGitDiffProvider extends GitDiffProvider { }); readonly isWorkingTreeCleanMock = vi.fn(async (): Promise => true); readonly getHeadCommitMock = vi.fn(async (): Promise => 'mock-sha'); + readonly disposeMock = vi.fn(() => undefined); constructor() { super('/repo'); @@ -138,6 +163,10 @@ class MockGitDiffProvider extends GitDiffProvider { override getHeadCommit(): Promise { return this.getHeadCommitMock(); } + + override dispose(): void { + this.disposeMock(); + } } function createMockGitDiffProvider(configure?: (provider: MockGitDiffProvider) => void) { @@ -168,6 +197,50 @@ function createStatus(value: Status['value'], data?: Status['data']): Status { }; } +/** + * Build a ReverseIndexImpl populated with the given (dep -> story -> depth) entries. + * Used by tests to control what `reverseIndex.lookup(changedFile)` returns. + */ +function buildReverseIndex(edges: Iterable): ReverseIndexImpl { + const reverseIndex = new ReverseIndexImpl(); + for (const [dep, story, depth] of edges) { + reverseIndex.record(dep, story, depth); + } + return reverseIndex; +} + +/** + * Stub the dependency-graph constructors so the service uses an in-test + * ReverseIndexImpl + an inert IncrementalPatcher. + * + * Note: `vi.mock` replaces these exports with plain `vi.fn()` constructors. When the + * service calls `new Ctor(...)` we must return objects via `mockImplementation` — + * but vitest invokes the impl with `Reflect.construct` on `new`, so arrow-function + * impls throw "is not a constructor". `function () { return obj; }` works because + * regular functions support `[[Construct]]`. + */ +function installDependencyGraphMocks(reverseIndex: ReverseIndexImpl): { + patchSpy: ReturnType; + buildSpy: ReturnType; +} { + const patchSpy = vi.fn(async () => undefined); + const buildSpy = vi.fn(async () => ({ reverseIndex, graph: new Map() })); + + vi.mocked(ChangeDetectionResolverFactory).mockImplementation(function () { + return { + resolve: vi.fn(async () => null), + } as unknown as ChangeDetectionResolverFactory; + } as unknown as new () => ChangeDetectionResolverFactory); + vi.mocked(DependencyGraphBuilder).mockImplementation(function () { + return { build: buildSpy } as unknown as DependencyGraphBuilder; + } as unknown as new () => DependencyGraphBuilder); + vi.mocked(IncrementalPatcher).mockImplementation(function () { + return { patch: patchSpy } as unknown as IncrementalPatcher; + } as unknown as new () => IncrementalPatcher); + + return { patchSpy, buildSpy }; +} + describe('ChangeDetectionService', () => { const workingDir = '/repo'; @@ -177,36 +250,28 @@ describe('ChangeDetectionService', () => { vi.mocked(logger.info).mockImplementation(() => undefined); vi.mocked(logger.warn).mockImplementation(() => undefined); vi.mocked(logger.error).mockImplementation(() => undefined); + vi.mocked(logger.debug).mockImplementation(() => undefined); }); afterEach(() => { vi.useRealTimers(); - vi.clearAllMocks(); + vi.resetAllMocks(); internal_resetChangeDetectionReadiness(); }); - it('marks only the nearest stories as modified', async () => { - const buttonCss = createModuleNode('/repo/src/Button.module.css'); - const buttonComponent = createModuleNode('/repo/src/Button.tsx'); - const buttonStory = createModuleNode('/repo/src/Button.stories.tsx'); - const headerComponent = createModuleNode('/repo/src/Header.tsx'); - const headerStory = createModuleNode('/repo/src/Header.stories.tsx'); - - buttonCss.importers.add(buttonComponent); - buttonComponent.importers.add(buttonStory); - buttonComponent.importers.add(headerComponent); - headerComponent.importers.add(headerStory); - - const moduleGraph: ModuleGraph = new Map([ - ['/repo/src/Button.module.css', new Set([buttonCss])], - ['/repo/src/Button.tsx', new Set([buttonComponent])], - ['/repo/src/Button.stories.tsx', new Set([buttonStory])], - ['/repo/src/Header.tsx', new Set([headerComponent])], - ['/repo/src/Header.stories.tsx', new Set([headerStory])], + it('edits a story file -> that story is modified at distance 0; importer stories are affected at distance 1', async () => { + // Story A is the changed file (distance 0). Story B imports A (distance 1). + // Reverse index models forward-walk depths: A reaches itself at 0; B reaches A at 1. + const reverseIndex = buildReverseIndex([ + ['/repo/src/A.stories.tsx', '/repo/src/A.stories.tsx', 0], + ['/repo/src/A.stories.tsx', '/repo/src/B.stories.tsx', 1], + ['/repo/src/B.stories.tsx', '/repo/src/B.stories.tsx', 0], ]); + installDependencyGraphMocks(reverseIndex); + const storyIndex = createStoryIndex([ - { storyId: 'button--primary', importPath: './src/Button.stories.tsx', title: 'Button' }, - { storyId: 'header--default', importPath: './src/Header.stories.tsx', title: 'Header' }, + { storyId: 'a--default', importPath: './src/A.stories.tsx', title: 'A' }, + { storyId: 'b--default', importPath: './src/B.stories.tsx', title: 'B' }, ]); const { getStatusStoreByTypeId } = createStatusStore({ universalStatusStore: new MockUniversalStore(UNIVERSAL_STATUS_STORE_OPTIONS), @@ -214,11 +279,11 @@ describe('ChangeDetectionService', () => { }); const gitDiffProvider = createMockGitDiffProvider((provider) => { provider.getChangedFilesMock.mockResolvedValue({ - changed: new Set(['src/Button.module.css']), + changed: new Set(['src/A.stories.tsx']), new: new Set(), }); }); - const { builder, emit } = createBuilder(); + const { adapter } = createMockAdapter(); const service = new ChangeDetectionService({ storyIndexGeneratorPromise: Promise.resolve({ getIndex: vi.fn().mockResolvedValue(storyIndex), @@ -229,14 +294,13 @@ describe('ChangeDetectionService', () => { workingDir, }); - service.start(builder.onModuleGraphChange, true); - emit(moduleGraph); + service.start(adapter, true); await vi.runAllTimersAsync(); expect(getStatusStoreByTypeId(CHANGE_DETECTION_STATUS_TYPE_ID).getAll()).toEqual({ - 'button--primary': { + 'a--default': { [CHANGE_DETECTION_STATUS_TYPE_ID]: { - storyId: 'button--primary', + storyId: 'a--default', typeId: CHANGE_DETECTION_STATUS_TYPE_ID, value: 'status-value:modified', title: '', @@ -244,9 +308,9 @@ describe('ChangeDetectionService', () => { sidebarContextMenu: false, }, }, - 'header--default': { + 'b--default': { [CHANGE_DETECTION_STATUS_TYPE_ID]: { - storyId: 'header--default', + storyId: 'b--default', typeId: CHANGE_DETECTION_STATUS_TYPE_ID, value: 'status-value:affected', title: '', @@ -255,11 +319,156 @@ describe('ChangeDetectionService', () => { }, }, }); + await service.dispose(); + }); + + it('edits a non-story dep at distance 1 from one story and distance 2 from another -> nearest is modified, farther is affected', async () => { + // Button.tsx is imported by Button.stories.tsx (distance 1) and by Compositions.stories.tsx + // transitively via the Button story chain (distance 2). + const reverseIndex = buildReverseIndex([ + ['/repo/src/Button.tsx', '/repo/src/Button.stories.tsx', 1], + ['/repo/src/Button.tsx', '/repo/src/Compositions.stories.tsx', 2], + ]); + installDependencyGraphMocks(reverseIndex); + + const storyIndex = createStoryIndex([ + { + storyId: 'button--primary', + importPath: './src/Button.stories.tsx', + title: 'Button', + }, + { + storyId: 'compositions--default', + importPath: './src/Compositions.stories.tsx', + title: 'Compositions', + }, + ]); + const { getStatusStoreByTypeId } = createStatusStore({ + universalStatusStore: new MockUniversalStore(UNIVERSAL_STATUS_STORE_OPTIONS), + environment: 'server', + }); + const gitDiffProvider = createMockGitDiffProvider((provider) => { + provider.getChangedFilesMock.mockResolvedValue({ + changed: new Set(['src/Button.tsx']), + new: new Set(), + }); + }); + const { adapter } = createMockAdapter(); + const service = new ChangeDetectionService({ + storyIndexGeneratorPromise: Promise.resolve({ + getIndex: vi.fn().mockResolvedValue(storyIndex), + } as never), + statusStore: getStatusStoreByTypeId(CHANGE_DETECTION_STATUS_TYPE_ID), + gitDiffProvider, + indexBaselineService: createMockStoryIndexBaselineService(), + workingDir, + }); + + service.start(adapter, true); + await vi.runAllTimersAsync(); + + const all = getStatusStoreByTypeId(CHANGE_DETECTION_STATUS_TYPE_ID).getAll(); + expect(all['button--primary'][CHANGE_DETECTION_STATUS_TYPE_ID].value).toBe( + 'status-value:modified' + ); + expect(all['compositions--default'][CHANGE_DETECTION_STATUS_TYPE_ID].value).toBe( + 'status-value:affected' + ); + await service.dispose(); + }); + + it('edits a non-story dep at equal distance from two stories -> both stories tie and are both modified', async () => { + // Both Button.stories.tsx and Header.stories.tsx import shared.ts at distance 1. + const reverseIndex = buildReverseIndex([ + ['/repo/src/shared.ts', '/repo/src/Button.stories.tsx', 1], + ['/repo/src/shared.ts', '/repo/src/Header.stories.tsx', 1], + ]); + installDependencyGraphMocks(reverseIndex); + + const storyIndex = createStoryIndex([ + { storyId: 'button--primary', importPath: './src/Button.stories.tsx', title: 'Button' }, + { storyId: 'header--default', importPath: './src/Header.stories.tsx', title: 'Header' }, + ]); + const { getStatusStoreByTypeId } = createStatusStore({ + universalStatusStore: new MockUniversalStore(UNIVERSAL_STATUS_STORE_OPTIONS), + environment: 'server', + }); + const gitDiffProvider = createMockGitDiffProvider((provider) => { + provider.getChangedFilesMock.mockResolvedValue({ + changed: new Set(['src/shared.ts']), + new: new Set(), + }); + }); + const { adapter } = createMockAdapter(); + const service = new ChangeDetectionService({ + storyIndexGeneratorPromise: Promise.resolve({ + getIndex: vi.fn().mockResolvedValue(storyIndex), + } as never), + statusStore: getStatusStoreByTypeId(CHANGE_DETECTION_STATUS_TYPE_ID), + gitDiffProvider, + indexBaselineService: createMockStoryIndexBaselineService(), + workingDir, + }); + + service.start(adapter, true); + await vi.runAllTimersAsync(); + + const all = getStatusStoreByTypeId(CHANGE_DETECTION_STATUS_TYPE_ID).getAll(); + expect(all['button--primary'][CHANGE_DETECTION_STATUS_TYPE_ID].value).toBe( + 'status-value:modified' + ); + expect(all['header--default'][CHANGE_DETECTION_STATUS_TYPE_ID].value).toBe( + 'status-value:modified' + ); + await service.dispose(); + }); + + it('edits a non-story file with no story importers -> reverse-index lookup is empty -> no status emitted', async () => { + // orphan.ts is in neither the reverse index nor the story index. + const reverseIndex = buildReverseIndex([ + ['/repo/src/Button.tsx', '/repo/src/Button.stories.tsx', 1], + ]); + installDependencyGraphMocks(reverseIndex); + + const storyIndex = createStoryIndex([ + { storyId: 'button--primary', importPath: './src/Button.stories.tsx', title: 'Button' }, + ]); + const { getStatusStoreByTypeId } = createStatusStore({ + universalStatusStore: new MockUniversalStore(UNIVERSAL_STATUS_STORE_OPTIONS), + environment: 'server', + }); + const gitDiffProvider = createMockGitDiffProvider((provider) => { + provider.getChangedFilesMock.mockResolvedValue({ + changed: new Set(['src/orphan.ts']), + new: new Set(), + }); + }); + const { adapter } = createMockAdapter(); + const service = new ChangeDetectionService({ + storyIndexGeneratorPromise: Promise.resolve({ + getIndex: vi.fn().mockResolvedValue(storyIndex), + } as never), + statusStore: getStatusStoreByTypeId(CHANGE_DETECTION_STATUS_TYPE_ID), + gitDiffProvider, + indexBaselineService: createMockStoryIndexBaselineService(), + workingDir, + }); + + service.start(adapter, true); + await vi.runAllTimersAsync(); + + expect(getStatusStoreByTypeId(CHANGE_DETECTION_STATUS_TYPE_ID).getAll()).toEqual({}); expect(await getChangeDetectionReadiness()).toEqual({ status: 'ready' }); await service.dispose(); }); + // ------------------------------------------------------------------ + // Orchestration / merge-status / readiness / git-state / debounce tests. + // ------------------------------------------------------------------ + it('marks new story files from the git new set and unsets them after they are reverted', async () => { + installDependencyGraphMocks(buildReverseIndex([])); + const storyIndex = createStoryIndex([ { storyId: 'new-button--primary', @@ -282,7 +491,11 @@ describe('ChangeDetectionService', () => { new: new Set(), }); }); - const { builder, emit } = createBuilder(); + let onGitStateChange: (() => void) | undefined; + gitDiffProvider.onGitStateChangeMock.mockImplementation((callback: () => void) => { + onGitStateChange = callback; + }); + const { adapter } = createMockAdapter(); const service = new ChangeDetectionService({ storyIndexGeneratorPromise: Promise.resolve({ getIndex: vi.fn().mockResolvedValue(storyIndex), @@ -294,8 +507,7 @@ describe('ChangeDetectionService', () => { debounceMs: 10, }); - service.start(builder.onModuleGraphChange, true); - emit(new Map()); + service.start(adapter, true); await vi.runAllTimersAsync(); expect(getStatusStoreByTypeId(CHANGE_DETECTION_STATUS_TYPE_ID).getAll()).toEqual({ @@ -311,7 +523,7 @@ describe('ChangeDetectionService', () => { }, }); - emit(new Map()); + onGitStateChange?.(); await vi.runAllTimersAsync(); expect(getStatusStoreByTypeId(CHANGE_DETECTION_STATUS_TYPE_ID).getAll()).toEqual({ @@ -321,18 +533,12 @@ describe('ChangeDetectionService', () => { }); it('replaces prior scan status data instead of cumulatively merging with store state', async () => { - const depA = createModuleNode('/repo/src/depA.ts'); - const depB = createModuleNode('/repo/src/depB.ts'); - const buttonStory = createModuleNode('/repo/src/Button.stories.tsx'); - - depA.importers.add(buttonStory); - depB.importers.add(buttonStory); - - const moduleGraph: ModuleGraph = new Map([ - ['/repo/src/depA.ts', new Set([depA])], - ['/repo/src/depB.ts', new Set([depB])], - ['/repo/src/Button.stories.tsx', new Set([buttonStory])], + const reverseIndex = buildReverseIndex([ + ['/repo/src/depB.ts', '/repo/src/Button.stories.tsx', 1], + ['/repo/src/depA.ts', '/repo/src/Button.stories.tsx', 1], ]); + installDependencyGraphMocks(reverseIndex); + const storyIndex = createStoryIndex([ { storyId: 'button--primary', importPath: './src/Button.stories.tsx', title: 'Button' }, ]); @@ -351,7 +557,11 @@ describe('ChangeDetectionService', () => { new: new Set(), }); }); - const { builder, emit } = createBuilder(); + let onGitStateChange: (() => void) | undefined; + gitDiffProvider.onGitStateChangeMock.mockImplementation((callback: () => void) => { + onGitStateChange = callback; + }); + const { adapter } = createMockAdapter(); const service = new ChangeDetectionService({ storyIndexGeneratorPromise: Promise.resolve({ getIndex: vi.fn().mockResolvedValue(storyIndex), @@ -363,8 +573,7 @@ describe('ChangeDetectionService', () => { debounceMs: 10, }); - service.start(builder.onModuleGraphChange, true); - emit(moduleGraph); + service.start(adapter, true); await vi.runAllTimersAsync(); expect(getStatusStoreByTypeId(CHANGE_DETECTION_STATUS_TYPE_ID).getAll()).toEqual({ @@ -380,7 +589,7 @@ describe('ChangeDetectionService', () => { }, }); - emit(moduleGraph); + onGitStateChange?.(); await vi.runAllTimersAsync(); expect(getStatusStoreByTypeId(CHANGE_DETECTION_STATUS_TYPE_ID).getAll()).toEqual({ @@ -399,10 +608,11 @@ describe('ChangeDetectionService', () => { }); it('rescans on git state changes using the normal debounce', async () => { - const buttonStory = createModuleNode('/repo/src/Button.stories.tsx'); - const moduleGraph: ModuleGraph = new Map([ - ['/repo/src/Button.stories.tsx', new Set([buttonStory])], + const reverseIndex = buildReverseIndex([ + ['/repo/src/Button.stories.tsx', '/repo/src/Button.stories.tsx', 0], ]); + installDependencyGraphMocks(reverseIndex); + const storyIndex = createStoryIndex([ { storyId: 'button--primary', importPath: './src/Button.stories.tsx', title: 'Button' }, ]); @@ -420,7 +630,7 @@ describe('ChangeDetectionService', () => { onGitStateChange = callback; }); }); - const { builder, emit } = createBuilder(); + const { adapter } = createMockAdapter(); const service = new ChangeDetectionService({ storyIndexGeneratorPromise: Promise.resolve({ getIndex: vi.fn().mockResolvedValue(storyIndex), @@ -432,8 +642,7 @@ describe('ChangeDetectionService', () => { debounceMs: 10, }); - service.start(builder.onModuleGraphChange, true); - emit(moduleGraph); + service.start(adapter, true); await vi.runAllTimersAsync(); expect(gitDiffProvider.onGitStateChangeMock).toHaveBeenCalledTimes(1); @@ -451,13 +660,58 @@ describe('ChangeDetectionService', () => { await service.dispose(); }); + it('debounces consecutive file-change events into a single scan', async () => { + installDependencyGraphMocks(buildReverseIndex([])); + + const storyIndex = createStoryIndex([ + { storyId: 'button--primary', importPath: './src/Button.stories.tsx', title: 'Button' }, + ]); + const { getStatusStoreByTypeId } = createStatusStore({ + universalStatusStore: new MockUniversalStore(UNIVERSAL_STATUS_STORE_OPTIONS), + environment: 'server', + }); + const gitDiffProvider = createMockGitDiffProvider(); + const { adapter, emitFileChange } = createMockAdapter(); + const service = new ChangeDetectionService({ + storyIndexGeneratorPromise: Promise.resolve({ + getIndex: vi.fn().mockResolvedValue(storyIndex), + } as never), + statusStore: getStatusStoreByTypeId(CHANGE_DETECTION_STATUS_TYPE_ID), + gitDiffProvider, + indexBaselineService: createMockStoryIndexBaselineService(), + workingDir, + debounceMs: 50, + }); + + service.start(adapter, true); + // First scan from initial start — debounce 0 runs synchronously. + await vi.runAllTimersAsync(); + expect(gitDiffProvider.getChangedFilesMock).toHaveBeenCalledTimes(1); + + // Three file-change events within the debounce window collapse to one scan. + emitFileChange({ kind: 'change', path: '/repo/src/A.ts' }); + await vi.advanceTimersByTimeAsync(10); + emitFileChange({ kind: 'change', path: '/repo/src/B.ts' }); + await vi.advanceTimersByTimeAsync(10); + emitFileChange({ kind: 'change', path: '/repo/src/C.ts' }); + await vi.advanceTimersByTimeAsync(10); + + expect(gitDiffProvider.getChangedFilesMock).toHaveBeenCalledTimes(1); + + await vi.advanceTimersByTimeAsync(50); + + expect(gitDiffProvider.getChangedFilesMock).toHaveBeenCalledTimes(2); + + await service.dispose(); + }); + it('does not subscribe to git state when change detection is disabled', async () => { const { getStatusStoreByTypeId } = createStatusStore({ universalStatusStore: new MockUniversalStore(UNIVERSAL_STATUS_STORE_OPTIONS), environment: 'server', }); const gitDiffProvider = createMockGitDiffProvider(); - const { builder } = createBuilder(); + const { adapter, hasFileChangeSubscriber } = createMockAdapter(); const service = new ChangeDetectionService({ storyIndexGeneratorPromise: Promise.resolve({ getIndex: vi.fn(), @@ -468,9 +722,9 @@ describe('ChangeDetectionService', () => { workingDir, }); - service.start(builder.onModuleGraphChange, false); + service.start(adapter, false); - expect(builder.onModuleGraphChange).not.toHaveBeenCalled(); + expect(hasFileChangeSubscriber()).toBe(false); expect(gitDiffProvider.onGitStateChangeMock).not.toHaveBeenCalled(); expect(await getChangeDetectionReadiness()).toEqual({ status: 'unavailable', @@ -479,7 +733,7 @@ describe('ChangeDetectionService', () => { await service.dispose(); }); - it('logs unavailability when the builder does not expose module graph changes', async () => { + it('logs unavailability when the builder does not provide an adapter', async () => { const { getStatusStoreByTypeId } = createStatusStore({ universalStatusStore: new MockUniversalStore(UNIVERSAL_STATUS_STORE_OPTIONS), environment: 'server', @@ -497,25 +751,31 @@ describe('ChangeDetectionService', () => { service.start(undefined, true); expect(logger.warn).toHaveBeenCalledWith( - 'Change detection unavailable: Not supported by builder' + 'Change detection unavailable: builder does not support change detection' ); expect(await getChangeDetectionReadiness()).toEqual({ status: 'unavailable', - reason: 'builder does not support module graph', + reason: 'builder does not support change detection', }); await service.dispose(); }); - it('resolves readiness when the builder reports change detection startup failure', async () => { + it('resolves readiness as unavailable when the adapter reports a startup failure', async () => { + // Park git lookup so the initial scan never resolves to 'ready' before we emit the failure. + const gitDeferred = createDeferred(); + installDependencyGraphMocks(buildReverseIndex([])); + const { getStatusStoreByTypeId } = createStatusStore({ universalStatusStore: new MockUniversalStore(UNIVERSAL_STATUS_STORE_OPTIONS), environment: 'server', }); - const gitDiffProvider = createMockGitDiffProvider(); - const { builder, emitError } = createBuilder(); + const gitDiffProvider = createMockGitDiffProvider((provider) => { + provider.getChangedFilesMock.mockImplementation(() => gitDeferred.promise); + }); + const { adapter, emitStartupFailure } = createMockAdapter(); const service = new ChangeDetectionService({ storyIndexGeneratorPromise: Promise.resolve({ - getIndex: vi.fn(), + getIndex: vi.fn().mockResolvedValue(createStoryIndex([])), } as never), statusStore: getStatusStoreByTypeId(CHANGE_DETECTION_STATUS_TYPE_ID), gitDiffProvider, @@ -523,22 +783,98 @@ describe('ChangeDetectionService', () => { workingDir, }); - service.start(builder.onModuleGraphChange, true); - emitError(new Error('module graph warmup failed')); - await Promise.resolve(); + service.start(adapter, true); + // Let startInternal subscribe before emitting the failure (initial scan parked on git). + await vi.runAllTimersAsync(); + + emitStartupFailure({ reason: 'vite warmup failed', error: new Error('warmup failed') }); + await vi.runAllTimersAsync(); + + expect(logger.warn).toHaveBeenCalledWith('Change detection unavailable: vite warmup failed'); + expect(await getChangeDetectionReadiness()).toEqual({ + status: 'unavailable', + reason: 'vite warmup failed', + error: expect.objectContaining({ message: 'warmup failed' }), + }); + // Unblock the parked git call so dispose can drain. + gitDeferred.resolve({ changed: new Set(), new: new Set() }); + await service.dispose(); + }); + + it('resolves readiness as error when the eager build throws', async () => { + const { buildSpy } = installDependencyGraphMocks(buildReverseIndex([])); + buildSpy.mockImplementation(async () => { + throw new ChangeDetectionFailureError('graph build blew up'); + }); + + const { getStatusStoreByTypeId } = createStatusStore({ + universalStatusStore: new MockUniversalStore(UNIVERSAL_STATUS_STORE_OPTIONS), + environment: 'server', + }); + const { adapter } = createMockAdapter(); + const service = new ChangeDetectionService({ + storyIndexGeneratorPromise: Promise.resolve({ + getIndex: vi.fn().mockResolvedValue(createStoryIndex([])), + } as never), + statusStore: getStatusStoreByTypeId(CHANGE_DETECTION_STATUS_TYPE_ID), + gitDiffProvider: createMockGitDiffProvider(), + indexBaselineService: createMockStoryIndexBaselineService(), + workingDir, + }); + + service.start(adapter, true); + await vi.runAllTimersAsync(); expect(logger.error).toHaveBeenCalledWith( - 'Change detection failed: module graph warmup failed' + 'Change detection failed to start: graph build blew up' ); expect(await getChangeDetectionReadiness()).toEqual({ status: 'error', - error: expect.objectContaining({ message: 'module graph warmup failed' }), + error: expect.objectContaining({ message: 'graph build blew up' }), }); - expect(gitDiffProvider.getChangedFilesMock).not.toHaveBeenCalled(); + await service.dispose(); + }); + + it('disposes the pool when startInternal throws', async () => { + // The startInternal pipeline throws after startup. Without the dispose() call in the + // catch handler disposeOxcParsePool would not be called. + const { buildSpy } = installDependencyGraphMocks(buildReverseIndex([])); + buildSpy.mockImplementation(async () => { + throw new ChangeDetectionFailureError('graph build blew up'); + }); + + const disposePoolSpy = vi.spyOn(oxcParser, 'disposeOxcParsePool').mockResolvedValue(undefined); + + const { getStatusStoreByTypeId } = createStatusStore({ + universalStatusStore: new MockUniversalStore(UNIVERSAL_STATUS_STORE_OPTIONS), + environment: 'server', + }); + const { adapter } = createMockAdapter(); + const service = new ChangeDetectionService({ + storyIndexGeneratorPromise: Promise.resolve({ + getIndex: vi.fn().mockResolvedValue(createStoryIndex([])), + } as never), + statusStore: getStatusStoreByTypeId(CHANGE_DETECTION_STATUS_TYPE_ID), + gitDiffProvider: createMockGitDiffProvider(), + indexBaselineService: createMockStoryIndexBaselineService(), + workingDir, + }); + + service.start(adapter, true); + await vi.runAllTimersAsync(); + + // dispose() must have been called, which calls disposeOxcParsePool exactly once. + expect(disposePoolSpy).toHaveBeenCalledTimes(1); + await service.dispose(); }); it('keeps the previous statuses when a live rescan fails', async () => { + const reverseIndex = buildReverseIndex([ + ['/repo/src/Button.stories.tsx', '/repo/src/Button.stories.tsx', 0], + ]); + installDependencyGraphMocks(reverseIndex); + const storyIndex = createStoryIndex([ { storyId: 'button--primary', importPath: './src/Button.stories.tsx', title: 'Button' }, ]); @@ -554,7 +890,11 @@ describe('ChangeDetectionService', () => { }) .mockRejectedValueOnce(new ChangeDetectionFailureError('scan blew up')); }); - const { builder, emit } = createBuilder(); + let onGitStateChange: (() => void) | undefined; + gitDiffProvider.onGitStateChangeMock.mockImplementation((callback: () => void) => { + onGitStateChange = callback; + }); + const { adapter } = createMockAdapter(); const service = new ChangeDetectionService({ storyIndexGeneratorPromise: Promise.resolve({ getIndex: vi.fn().mockResolvedValue(storyIndex), @@ -566,10 +906,10 @@ describe('ChangeDetectionService', () => { debounceMs: 10, }); - service.start(builder.onModuleGraphChange, true); - emit(new Map()); + service.start(adapter, true); await vi.runAllTimersAsync(); - emit(new Map()); + + onGitStateChange?.(); await vi.runAllTimersAsync(); expect(getStatusStoreByTypeId(CHANGE_DETECTION_STATUS_TYPE_ID).getAll()).toEqual({ @@ -589,13 +929,14 @@ describe('ChangeDetectionService', () => { }); it('does not apply scan results or rerun after disposal', async () => { + const reverseIndex = buildReverseIndex([ + ['/repo/src/Button.stories.tsx', '/repo/src/Button.stories.tsx', 0], + ]); + installDependencyGraphMocks(reverseIndex); + const storyIndex = createStoryIndex([ { storyId: 'button--primary', importPath: './src/Button.stories.tsx', title: 'Button' }, ]); - const buttonStory = createModuleNode('/repo/src/Button.stories.tsx'); - const moduleGraph: ModuleGraph = new Map([ - ['/repo/src/Button.stories.tsx', new Set([buttonStory])], - ]); const { getStatusStoreByTypeId } = createStatusStore({ universalStatusStore: new MockUniversalStore(UNIVERSAL_STATUS_STORE_OPTIONS), environment: 'server', @@ -607,7 +948,7 @@ describe('ChangeDetectionService', () => { const gitDiffProvider = createMockGitDiffProvider((provider) => { provider.getChangedFilesMock.mockImplementation(() => changedFilesDeferred.promise); }); - const { builder, emit } = createBuilder(); + const { adapter } = createMockAdapter(); const service = new ChangeDetectionService({ storyIndexGeneratorPromise: Promise.resolve({ getIndex: vi.fn().mockResolvedValue(storyIndex), @@ -619,10 +960,7 @@ describe('ChangeDetectionService', () => { debounceMs: 0, }); - service.start(builder.onModuleGraphChange, true); - emit(moduleGraph); - await vi.advanceTimersByTimeAsync(0); - emit(moduleGraph); + service.start(adapter, true); await vi.advanceTimersByTimeAsync(0); await service.dispose(); @@ -633,7 +971,6 @@ describe('ChangeDetectionService', () => { await Promise.resolve(); await Promise.resolve(); - expect(gitDiffProvider.getChangedFilesMock).toHaveBeenCalledTimes(1); expect(getStatusStoreByTypeId(CHANGE_DETECTION_STATUS_TYPE_ID).getAll()).toEqual({}); await expect( Promise.race([ @@ -644,6 +981,8 @@ describe('ChangeDetectionService', () => { }); it('tears down after a permanently unavailable scan result', async () => { + installDependencyGraphMocks(buildReverseIndex([])); + const { getStatusStoreByTypeId } = createStatusStore({ universalStatusStore: new MockUniversalStore(UNIVERSAL_STATUS_STORE_OPTIONS), environment: 'server', @@ -653,10 +992,10 @@ describe('ChangeDetectionService', () => { new ChangeDetectionUnavailableError('not a git repository') ); }); - const { builder, emit } = createBuilder(); + const { adapter } = createMockAdapter(); const service = new ChangeDetectionService({ storyIndexGeneratorPromise: Promise.resolve({ - getIndex: vi.fn(), + getIndex: vi.fn().mockResolvedValue(createStoryIndex([])), } as never), statusStore: getStatusStoreByTypeId(CHANGE_DETECTION_STATUS_TYPE_ID), gitDiffProvider, @@ -665,11 +1004,8 @@ describe('ChangeDetectionService', () => { debounceMs: 0, }); - service.start(builder.onModuleGraphChange, true); - emit(new Map()); - await vi.advanceTimersByTimeAsync(0); - emit(new Map()); - await vi.advanceTimersByTimeAsync(0); + service.start(adapter, true); + await vi.runAllTimersAsync(); expect(gitDiffProvider.getChangedFilesMock).toHaveBeenCalledTimes(1); expect(logger.warn).toHaveBeenCalledWith('Change detection unavailable: not a git repository'); @@ -681,30 +1017,69 @@ describe('ChangeDetectionService', () => { await service.dispose(); }); - it('prefers modified over affected when the same story is reached by multiple changed files', async () => { - const shared = createModuleNode('/repo/src/shared.ts'); - const closeComponent = createModuleNode('/repo/src/Close.tsx'); - const farComponent = createModuleNode('/repo/src/Far.tsx'); - const story = createModuleNode('/repo/src/Button.stories.tsx'); - - shared.importers.add(closeComponent); - shared.importers.add(farComponent); - closeComponent.importers.add(story); - farComponent.importers.add(closeComponent); - - const directChange = createModuleNode('/repo/src/direct.ts'); - const indirectChange = createModuleNode('/repo/src/indirect.ts'); - directChange.importers.add(closeComponent); - indirectChange.importers.add(farComponent); - - const moduleGraph: ModuleGraph = new Map([ - ['/repo/src/shared.ts', new Set([shared])], - ['/repo/src/Close.tsx', new Set([closeComponent])], - ['/repo/src/Far.tsx', new Set([farComponent])], - ['/repo/src/Button.stories.tsx', new Set([story])], - ['/repo/src/direct.ts', new Set([directChange])], - ['/repo/src/indirect.ts', new Set([indirectChange])], + it('serialises concurrent file-change events through the patch chain', async () => { + const reverseIndex = buildReverseIndex([]); + const { patchSpy, buildSpy } = installDependencyGraphMocks(reverseIndex); + buildSpy.mockResolvedValue({ reverseIndex, graph: new Map() }); + + let activePatches = 0; + let maxConcurrent = 0; + patchSpy.mockImplementation(async () => { + activePatches += 1; + maxConcurrent = Math.max(maxConcurrent, activePatches); + // Force an actual await so two concurrent calls would visibly interleave. + await new Promise((resolve) => setImmediate(resolve)); + activePatches -= 1; + }); + + const storyIndex = createStoryIndex([ + { storyId: 'button--primary', importPath: './src/Button.stories.tsx', title: 'Button' }, ]); + const { getStatusStoreByTypeId } = createStatusStore({ + universalStatusStore: new MockUniversalStore(UNIVERSAL_STATUS_STORE_OPTIONS), + environment: 'server', + }); + const { adapter, emitFileChange } = createMockAdapter(); + const service = new ChangeDetectionService({ + storyIndexGeneratorPromise: Promise.resolve({ + getIndex: vi.fn().mockResolvedValue(storyIndex), + } as never), + statusStore: getStatusStoreByTypeId(CHANGE_DETECTION_STATUS_TYPE_ID), + gitDiffProvider: createMockGitDiffProvider(), + indexBaselineService: createMockStoryIndexBaselineService(), + workingDir, + }); + + service.start(adapter, true); + await vi.runAllTimersAsync(); + + emitFileChange({ kind: 'change', path: '/repo/src/Button.tsx' }); + emitFileChange({ kind: 'change', path: '/repo/src/Other.tsx' }); + emitFileChange({ kind: 'unlink', path: '/repo/src/Stale.tsx' }); + await vi.runAllTimersAsync(); + + expect(patchSpy).toHaveBeenCalledTimes(3); + expect(maxConcurrent).toBe(1); + + await service.dispose(); + }); + + it('scan waits for the current patch to settle before reading reverseIndex', async () => { + // Without the patchSnapshot await in scan(), a git-state-change that fires scheduleScan + // while a patch is mid-rewalk (reverseIndex transiently empty) reads the empty index + // and publishes no statuses even though the patch will eventually add entries back. + const reverseIndex = buildReverseIndex([]); + const patchDeferred = createDeferred(); + const { patchSpy, buildSpy } = installDependencyGraphMocks(reverseIndex); + buildSpy.mockResolvedValue({ reverseIndex, graph: new Map() }); + + // The first patch call blocks; during that block a scan is scheduled. After the patch + // resolves, reverseIndex has a real entry that the scan should see. + patchSpy.mockImplementationOnce(async () => { + await patchDeferred.promise; + reverseIndex.record('/repo/src/Button.stories.tsx', '/repo/src/Button.stories.tsx', 0); + }); + const storyIndex = createStoryIndex([ { storyId: 'button--primary', importPath: './src/Button.stories.tsx', title: 'Button' }, ]); @@ -714,11 +1089,15 @@ describe('ChangeDetectionService', () => { }); const gitDiffProvider = createMockGitDiffProvider((provider) => { provider.getChangedFilesMock.mockResolvedValue({ - changed: new Set(['src/direct.ts', 'src/indirect.ts']), + changed: new Set(['src/Button.stories.tsx']), new: new Set(), }); }); - const { builder, emit } = createBuilder(); + let triggerGitStateChange: (() => void) | undefined; + gitDiffProvider.onGitStateChangeMock.mockImplementation((callback: () => void) => { + triggerGitStateChange = callback; + }); + const { adapter, emitFileChange } = createMockAdapter(); const service = new ChangeDetectionService({ storyIndexGeneratorPromise: Promise.resolve({ getIndex: vi.fn().mockResolvedValue(storyIndex), @@ -727,43 +1106,38 @@ describe('ChangeDetectionService', () => { gitDiffProvider, indexBaselineService: createMockStoryIndexBaselineService(), workingDir, + debounceMs: 0, }); - service.start(builder.onModuleGraphChange, true); - emit(moduleGraph); + service.start(adapter, true); await vi.runAllTimersAsync(); - expect(getStatusStoreByTypeId(CHANGE_DETECTION_STATUS_TYPE_ID).getAll()).toEqual({ - 'button--primary': { - [CHANGE_DETECTION_STATUS_TYPE_ID]: { - storyId: 'button--primary', - typeId: CHANGE_DETECTION_STATUS_TYPE_ID, - value: 'status-value:modified', - title: '', - description: '', - sidebarContextMenu: false, - }, - }, - }); + // Start a patch that will block, then immediately schedule a scan mid-patch. + emitFileChange({ kind: 'change', path: '/repo/src/Button.stories.tsx' }); + triggerGitStateChange?.(); + + // Unblock the patch — now the reverseIndex has the entry. + patchDeferred.resolve(); + await vi.runAllTimersAsync(); + + // The scan that ran after the patch settled should have seen the populated reverseIndex. + const all = getStatusStoreByTypeId(CHANGE_DETECTION_STATUS_TYPE_ID).getAll(); + expect(all['button--primary']?.[CHANGE_DETECTION_STATUS_TYPE_ID]?.value).toBe( + 'status-value:modified' + ); + await service.dispose(); }); - it('handles normalized paths when assigning statuses', async () => { - const buttonCssPath = join(workingDir, 'src', 'Button.module.css'); - const buttonComponentPath = join(workingDir, 'src', 'Button.tsx'); - const buttonStoryPath = join(workingDir, 'src', 'Button.stories.tsx'); - const buttonCss = createModuleNode(buttonCssPath); - const buttonComponent = createModuleNode(buttonComponentPath); - const buttonStory = createModuleNode(buttonStoryPath); - - buttonCss.importers.add(buttonComponent); - buttonComponent.importers.add(buttonStory); - - const moduleGraph: ModuleGraph = new Map([ - [buttonCssPath, new Set([buttonCss])], - [buttonComponentPath, new Set([buttonComponent])], - [buttonStoryPath, new Set([buttonStory])], - ]); + it('does not patch file-change events emitted before the adapter subscription is installed (pre-build events are dropped)', async () => { + const reverseIndex = buildReverseIndex([]); + const buildDeferred = createDeferred(); + const { patchSpy, buildSpy } = installDependencyGraphMocks(reverseIndex); + buildSpy.mockImplementation(async () => { + await buildDeferred.promise; + return { reverseIndex, graph: new Map() }; + }); + const storyIndex = createStoryIndex([ { storyId: 'button--primary', importPath: './src/Button.stories.tsx', title: 'Button' }, ]); @@ -771,39 +1145,135 @@ describe('ChangeDetectionService', () => { universalStatusStore: new MockUniversalStore(UNIVERSAL_STATUS_STORE_OPTIONS), environment: 'server', }); - const gitDiffProvider = createMockGitDiffProvider((provider) => { - provider.getChangedFilesMock.mockResolvedValue({ - changed: new Set(['src/Button.module.css']), - new: new Set(), - }); - }); - const { builder, emit } = createBuilder(); + const { adapter, emitFileChange } = createMockAdapter(); const service = new ChangeDetectionService({ storyIndexGeneratorPromise: Promise.resolve({ getIndex: vi.fn().mockResolvedValue(storyIndex), } as never), statusStore: getStatusStoreByTypeId(CHANGE_DETECTION_STATUS_TYPE_ID), + gitDiffProvider: createMockGitDiffProvider(), + indexBaselineService: createMockStoryIndexBaselineService(), + workingDir, + }); + + service.start(adapter, true); + // Allow startInternal to reach the build step and start awaiting it. + await Promise.resolve(); + await Promise.resolve(); + + // The service subscribes to file-change events strictly after the eager build resolves, + // so anything emitted by the adapter before then has nowhere to land. Assert no patch + // calls have happened yet. + expect(patchSpy).not.toHaveBeenCalled(); + + buildDeferred.resolve(); + await vi.runAllTimersAsync(); + + // Now the adapter has subscribers — file events go through the patcher. + emitFileChange({ kind: 'change', path: '/repo/src/Button.tsx' }); + await vi.runAllTimersAsync(); + expect(patchSpy).toHaveBeenCalledTimes(1); + + await service.dispose(); + }); + + it('calls gitDiffProvider.dispose() on service dispose when a git watcher was installed', async () => { + // Watcher leak: onGitStateChange installs FS watchers; without dispose() the watchers + // survive the service lifetime and fire stale callbacks on long-lived processes. + installDependencyGraphMocks(buildReverseIndex([])); + + const gitDiffProvider = createMockGitDiffProvider((provider) => { + // Simulate having called onGitStateChange (watcher installed). + provider.onGitStateChangeMock.mockImplementation(() => undefined); + }); + const { adapter } = createMockAdapter(); + const service = new ChangeDetectionService({ + storyIndexGeneratorPromise: Promise.resolve({ + getIndex: vi.fn().mockResolvedValue(createStoryIndex([])), + } as never), + statusStore: createStatusStore({ + universalStatusStore: new MockUniversalStore(UNIVERSAL_STATUS_STORE_OPTIONS), + environment: 'server', + }).getStatusStoreByTypeId(CHANGE_DETECTION_STATUS_TYPE_ID), gitDiffProvider, indexBaselineService: createMockStoryIndexBaselineService(), workingDir, }); - service.start(builder.onModuleGraphChange, true); - emit(moduleGraph); + service.start(adapter, true); await vi.runAllTimersAsync(); - expect(getStatusStoreByTypeId(CHANGE_DETECTION_STATUS_TYPE_ID).getAll()).toEqual({ - 'button--primary': { - [CHANGE_DETECTION_STATUS_TYPE_ID]: { - storyId: 'button--primary', - typeId: CHANGE_DETECTION_STATUS_TYPE_ID, - value: 'status-value:modified', - title: '', - description: '', - sidebarContextMenu: false, - }, - }, + await service.dispose(); + + expect(gitDiffProvider.disposeMock).toHaveBeenCalledTimes(1); + }); + + it('does not call gitDiffProvider.dispose() when the provider was never constructed by the service', async () => { + // If gitDiffProvider is never passed in and start() exits early (disabled), the service + // never lazily constructs a provider, so dispose() must not create one just to tear it down. + const { getStatusStoreByTypeId } = createStatusStore({ + universalStatusStore: new MockUniversalStore(UNIVERSAL_STATUS_STORE_OPTIONS), + environment: 'server', + }); + const service = new ChangeDetectionService({ + storyIndexGeneratorPromise: Promise.resolve({ + getIndex: vi.fn(), + } as never), + statusStore: getStatusStoreByTypeId(CHANGE_DETECTION_STATUS_TYPE_ID), + // No gitDiffProvider injected — service would lazily create one, but start(false) exits + // before getGitDiffProvider() is called. + indexBaselineService: createMockStoryIndexBaselineService(), + workingDir, + }); + + service.start(undefined, false); + // Should not throw and should not attempt to call dispose on an unconstructed provider. + await expect(service.dispose()).resolves.toBeUndefined(); + }); + + it('replays add/unlink through the patcher when onStoryIndexInvalidated reveals new/removed stories', async () => { + const reverseIndex = buildReverseIndex([]); + const { patchSpy, buildSpy } = installDependencyGraphMocks(reverseIndex); + buildSpy.mockResolvedValue({ reverseIndex, graph: new Map() }); + + const initialIndex = createStoryIndex([ + { storyId: 'a--default', importPath: './src/A.stories.tsx', title: 'A' }, + ]); + const updatedIndex = createStoryIndex([ + { storyId: 'a--default', importPath: './src/A.stories.tsx', title: 'A' }, + { storyId: 'b--default', importPath: './src/B.stories.tsx', title: 'B' }, + ]); + const getIndex = vi + .fn() + .mockResolvedValueOnce(initialIndex) + .mockResolvedValueOnce(initialIndex) + .mockResolvedValue(updatedIndex); + + const { getStatusStoreByTypeId } = createStatusStore({ + universalStatusStore: new MockUniversalStore(UNIVERSAL_STATUS_STORE_OPTIONS), + environment: 'server', }); + const { adapter } = createMockAdapter(); + const service = new ChangeDetectionService({ + storyIndexGeneratorPromise: Promise.resolve({ getIndex } as never), + statusStore: getStatusStoreByTypeId(CHANGE_DETECTION_STATUS_TYPE_ID), + gitDiffProvider: createMockGitDiffProvider(), + indexBaselineService: createMockStoryIndexBaselineService(), + workingDir, + }); + + service.start(adapter, true); + await vi.runAllTimersAsync(); + expect(patchSpy).not.toHaveBeenCalled(); + + service.onStoryIndexInvalidated(); + await vi.runAllTimersAsync(); + + expect(patchSpy).toHaveBeenCalledWith({ + kind: 'add', + path: '/repo/src/B.stories.tsx', + }); + await service.dispose(); }); }); diff --git a/code/core/src/core-server/change-detection/ChangeDetectionService.ts b/code/core/src/core-server/change-detection/ChangeDetectionService.ts index 857275982690..40277b5ee7b2 100644 --- a/code/core/src/core-server/change-detection/ChangeDetectionService.ts +++ b/code/core/src/core-server/change-detection/ChangeDetectionService.ts @@ -1,11 +1,11 @@ -import { join } from 'pathe'; +import { join, normalize } from 'pathe'; +import { dequal } from 'dequal'; import { logger } from 'storybook/internal/node-logger'; +import { disposeOxcParsePool } from 'storybook/internal/oxc-parser'; +import { getProjectRoot } from 'storybook/internal/common'; import type { - Builder, - ModuleGraph, - ModuleGraphChangeEvent, - ModuleNode, + Presets, StatusValue, StoryIndex, Status, @@ -13,13 +13,21 @@ import type { } from 'storybook/internal/types'; import { CHANGE_DETECTION_STATUS_TYPE_ID } from 'storybook/internal/types'; -import { normalizePath } from '../../common/utils/normalize-path.ts'; import type { StoryIndexGenerator } from '../utils/StoryIndexGenerator.ts'; +import type { ChangeDetectionAdapter, FileChangeEvent } from './adapters/index.ts'; +import { + ChangeDetectionResolverFactory, + DependencyGraphBuilder, + IncrementalPatcher, + ParseResolveCache, +} from './dependency-graph/index.ts'; +import type { ReverseIndexImpl } from './dependency-graph/index.ts'; import { ChangeDetectionFailureError, ChangeDetectionUnavailableError } from './errors.ts'; import { GitDiffProvider } from './GitDiffProvider.ts'; -import { resetChangeDetectionReadiness, setChangeDetectionReadiness } from './readiness.ts'; import { extractBaselineEntryIds, IndexBaselineService } from './IndexBaselineService.ts'; -import { findAffectedStoryFiles } from './trace-changed.ts'; +import type { ImportParser } from './parser-registry/index.ts'; +import { ParserRegistry, builtinImportParsers } from './parser-registry/index.ts'; +import { resetChangeDetectionReadiness, setChangeDetectionReadiness } from './readiness.ts'; const CHANGE_DETECTION_DEBOUNCE_MS = 200; @@ -35,23 +43,34 @@ function isSameStatus(a: Status | undefined, b: Status): boolean { a.title === b.title && a.description === b.description && a.sidebarContextMenu === b.sidebarContextMenu && - JSON.stringify(a.data) === JSON.stringify(b.data) + dequal(a.data, b.data) ); } +type StoryIdsByFileCacheKey = Awaited>; +const storyIdsByFileCache = new WeakMap< + StoryIdsByFileCacheKey, + { workingDir: string; storyIdsByFile: Map> } +>(); + function getStoryIdsByAbsolutePath( - storyIndex: Awaited>, + storyIndex: StoryIdsByFileCacheKey, workingDir: string ): Map> { + const cached = storyIdsByFileCache.get(storyIndex); + if (cached && cached.workingDir === workingDir) { + return cached.storyIdsByFile; + } const storyIdsByFile = new Map>(); Object.values(storyIndex.entries).forEach((entry) => { if (entry.type === 'story' && !entry.importPath.startsWith('virtual:')) { - const filePath = join(workingDir, entry.importPath); + const filePath = normalize(join(workingDir, entry.importPath)); const storyIds = storyIdsByFile.get(filePath) ?? new Set(); storyIds.add(entry.id); storyIdsByFile.set(filePath, storyIds); } }); + storyIdsByFileCache.set(storyIndex, { workingDir, storyIdsByFile }); return storyIdsByFile; } @@ -115,16 +134,14 @@ export function buildIndexBaselineStatuses( } /** - * Coordinates change detection by listening to builder module-graph updates, resolving changed - * files from git, mapping those changes to affected stories, and publishing the resulting story - * statuses to the status store. + * Coordinates change detection by owning a builder-supplied {@link ChangeDetectionAdapter}, + * eagerly building a reverse-dependency index from story files at startup, applying + * file-system events incrementally to that index, resolving git-changed files, and publishing + * the resulting story statuses to the status store. */ export class ChangeDetectionService { private disposed = false; - private unsubscribeModuleGraph: (() => void) | undefined; private debounceTimer: ReturnType | undefined; - private latestModuleGraph: ModuleGraph | undefined; - private hasReceivedModuleGraph = false; private scanInFlight = false; private rerunAfterCurrentScan = false; private readinessResolved = false; @@ -133,6 +150,19 @@ export class ChangeDetectionService { private indexBaselineService: IndexBaselineService | undefined; private readonly workingDir: string; private readonly debounceMs: number; + private adapter: ChangeDetectionAdapter | undefined; + private dependencyGraphBuilder: DependencyGraphBuilder | undefined; + private incrementalPatcher: IncrementalPatcher | undefined; + private reverseIndex: ReverseIndexImpl | undefined; + private storyFiles: Set = new Set(); + /** + * Serialises file-change patches so two events touching the same dep set never interleave + * across `await` points inside `IncrementalPatcher.patch`. The chain ignores rejections + * (each call's failure is logged in {@link handleFileChange}). + */ + private patchQueue: Promise = Promise.resolve(); + private unsubscribeFileChange: (() => void) | undefined; + private unsubscribeStartupFailure: (() => void) | undefined; constructor( private readonly options: { @@ -142,6 +172,8 @@ export class ChangeDetectionService { indexBaselineService?: IndexBaselineService; workingDir?: string; debounceMs?: number; + /** Presets instance used to resolve `experimental_importParsers` contributions from framework/renderer plugins. */ + presets?: Presets; } ) { this.gitDiffProvider = options.gitDiffProvider; @@ -151,10 +183,7 @@ export class ChangeDetectionService { resetChangeDetectionReadiness(); } - start( - onModuleGraphChange: Builder['onModuleGraphChange'], - enabled: boolean | undefined - ): void { + start(adapter: ChangeDetectionAdapter | undefined, enabled: boolean | undefined): void { if (enabled === false) { logger.debug('Change detection disabled.'); this.resolveReadiness({ @@ -164,31 +193,126 @@ export class ChangeDetectionService { return; } - if (!onModuleGraphChange) { - logger.warn('Change detection unavailable: Not supported by builder'); + if (!adapter) { + logger.warn('Change detection unavailable: builder does not support change detection'); this.resolveReadiness({ status: 'unavailable', - reason: 'builder does not support module graph', + reason: 'builder does not support change detection', }); return; } logger.debug('Change detection enabled.'); - void this.getIndexBaselineService().start(); - this.unsubscribeModuleGraph = onModuleGraphChange((event) => { + this.adapter = adapter; + + void this.startInternal().catch((error) => { if (this.disposed) { return; } + const failure = + error instanceof Error ? error : new ChangeDetectionFailureError(String(error)); + logger.error(`Change detection failed to start: ${failure.message}`); + this.resolveReadiness({ status: 'error', error: failure }); + void this.dispose().catch(() => undefined); + }); + } - if (event.type === 'moduleGraph') { - this.latestModuleGraph = event.moduleGraph; - this.scheduleScan(this.hasReceivedModuleGraph ? this.debounceMs : 0); - this.hasReceivedModuleGraph = true; + /** + * Builds parser registry, resolver, dependency graph, and patcher; subscribes to + * file-change events queued behind {@link patchQueue}; kicks off the baseline service + * and initial scan. + */ + private async startInternal(): Promise { + const adapter = this.adapter; + if (!adapter) { + return; + } + + if (this.disposed) { + return; + } + + const resolveConfig = await adapter.getResolveConfig(); + const projectRoot = normalize(resolveConfig.projectRoot ?? this.workingDir); + + const pluginParsers = this.options.presets + ? await this.options.presets.apply('experimental_importParsers', []) + : []; + const registry = new ParserRegistry({ + defaultParsers: builtinImportParsers, + pluginParsers, + }); + const resolver = new ChangeDetectionResolverFactory(resolveConfig); + const workspaceRoots = new Set([normalize(getProjectRoot())]); + + const storyIndexGenerator = await this.options.storyIndexGeneratorPromise; + const storyIndex = await storyIndexGenerator.getIndex(); + const storyIdsByFile = getStoryIdsByAbsolutePath(storyIndex, this.workingDir); + this.storyFiles = new Set(storyIdsByFile.keys()); + + if (this.disposed) { + return; + } + + // Shared parse/resolve cache so the patcher reuses cold-start results instead of + // re-doing every file's parse + resolution on the first event after boot. The patcher + // invalidates per-file entries on every change/unlink before reading. + const cache = new ParseResolveCache({ + registry, + resolver, + workspaceRoots, + projectRoot, + logger, + }); + + this.dependencyGraphBuilder = new DependencyGraphBuilder({ + registry, + resolver, + workspaceRoots, + projectRoot, + cache, + }); + const { reverseIndex, graph } = await this.dependencyGraphBuilder.build(this.storyFiles); + if (this.disposed) { + return; + } + this.reverseIndex = reverseIndex; + + this.incrementalPatcher = new IncrementalPatcher({ + reverseIndex, + graph, + registry, + resolver, + workspaceRoots, + projectRoot, + cache, + isStoryFile: (path: string) => this.storyFiles.has(normalize(path)), + }); + + this.unsubscribeFileChange = adapter.onFileChange((event) => { + if (this.disposed) { return; } - - this.handleBuilderStartupEvent(event); + this.patchQueue = this.patchQueue.then(() => this.handleFileChange(event)); }); + + if (adapter.onStartupFailure) { + this.unsubscribeStartupFailure = adapter.onStartupFailure((event) => { + if (this.disposed) { + return; + } + logger.warn(`Change detection unavailable: ${event.reason}`); + this.resolveReadiness({ + status: 'unavailable', + reason: event.reason, + error: event.error, + }); + void this.dispose(); + }); + } + + void this.getIndexBaselineService().start(); + this.getGitDiffProvider().onGitStateChange(() => { if (this.disposed) { return; @@ -199,11 +323,64 @@ export class ChangeDetectionService { .handleGitStateChange() .catch(() => undefined); }); + + // Initial scan surfaces git-pending diffs immediately. + this.scheduleScan(0); } onStoryIndexInvalidated(): void { - if (!this.disposed) { - this.scheduleScan(this.debounceMs); + if (this.disposed) { + return; + } + void this.refreshStoryFiles().catch(() => undefined); + this.scheduleScan(this.debounceMs); + } + + /** + * Re-reads the story index and reconciles {@link storyFiles} with stories that have + * appeared or disappeared since startup. For each story that newly entered the index, the + * patcher is asked to walk it (so its forward edges are recorded). For each story that + * left the index, the patcher is asked to unlink it (so its reverse-index entries are + * pruned). Replays are queued behind {@link patchQueue} to keep the serialised-patch + * invariant intact. + */ + private async refreshStoryFiles(): Promise { + if (!this.incrementalPatcher) { + return; + } + const storyIndexGenerator = await this.options.storyIndexGeneratorPromise; + const storyIndex = await storyIndexGenerator.getIndex(); + if (this.disposed) { + return; + } + const storyIdsByFile = getStoryIdsByAbsolutePath(storyIndex, this.workingDir); + const next = new Set(storyIdsByFile.keys()); + const previous = this.storyFiles; + + const added: string[] = []; + for (const path of next) { + if (!previous.has(path)) { + added.push(path); + } + } + const removed: string[] = []; + for (const path of previous) { + if (!next.has(path)) { + removed.push(path); + } + } + + if (added.length === 0 && removed.length === 0) { + return; + } + + this.storyFiles = next; + + for (const path of added) { + this.patchQueue = this.patchQueue.then(() => this.handleFileChange({ kind: 'add', path })); + } + for (const path of removed) { + this.patchQueue = this.patchQueue.then(() => this.handleFileChange({ kind: 'unlink', path })); } } @@ -216,8 +393,33 @@ export class ChangeDetectionService { this.debounceTimer = undefined; } - this.unsubscribeModuleGraph?.(); - this.unsubscribeModuleGraph = undefined; + this.unsubscribeFileChange?.(); + this.unsubscribeFileChange = undefined; + this.unsubscribeStartupFailure?.(); + this.unsubscribeStartupFailure = undefined; + + this.gitDiffProvider?.dispose(); + // Drain in-flight patches before tearing down the OXC parse pool so no + // patch reads the pool after it has been disposed. + await this.patchQueue.catch(() => undefined); + await disposeOxcParsePool().catch(() => undefined); + } + + private async handleFileChange(event: FileChangeEvent): Promise { + if (this.disposed || !this.incrementalPatcher) { + return; + } + try { + await this.incrementalPatcher.patch(event); + } catch (error) { + logger.warn( + `Change detection: failed to apply ${event.kind} for ${event.path}: ${error instanceof Error ? error.message : String(error)}` + ); + } + if (this.disposed) { + return; + } + this.scheduleScan(this.debounceMs); } private scheduleScan(delayMs: number): void { @@ -232,7 +434,17 @@ export class ChangeDetectionService { } private async scan(): Promise { - if (this.disposed || !this.latestModuleGraph) { + if (this.disposed || !this.reverseIndex) { + return; + } + + // Snapshot and drain the current patch chain before reading reverseIndex. Without this + // await, a scan triggered mid-patch (between removeStory and the re-walk's recordEdges) + // reads a transiently empty reverseIndex and publishes incorrect statuses. + const patchSnapshot = this.patchQueue; + await patchSnapshot.catch(() => undefined); + + if (this.disposed || !this.reverseIndex) { return; } @@ -244,7 +456,7 @@ export class ChangeDetectionService { this.scanInFlight = true; try { - const nextStatuses = await this.buildStatuses(this.latestModuleGraph); + const nextStatuses = await this.buildStatuses(this.reverseIndex); if (this.disposed) { return; } @@ -291,7 +503,7 @@ export class ChangeDetectionService { } } - private async buildStatuses(moduleGraph: ModuleGraph): Promise> { + private async buildStatuses(reverseIndex: ReverseIndexImpl): Promise> { const gitDiffProvider = this.getGitDiffProvider(); const [changes, repoRoot, storyIndexGenerator, baselineEntryIds] = await Promise.all([ gitDiffProvider.getChangedFiles(), @@ -300,19 +512,13 @@ export class ChangeDetectionService { this.getIndexBaselineService().getBaselineEntryIds(), ]); - const changedFiles = new Set(Array.from(changes.changed).map((path) => join(repoRoot, path))); - const newFiles = new Set(Array.from(changes.new).map((path) => join(repoRoot, path))); + const changedFiles = new Set( + Array.from(changes.changed).map((path) => normalize(join(repoRoot, path))) + ); + const newFiles = new Set( + Array.from(changes.new).map((path) => normalize(join(repoRoot, path))) + ); const scannedFiles = new Set([...changedFiles, ...newFiles]); - const normalizedModuleGraph = new Map>(); - moduleGraph.forEach((nodes, filePath) => { - const normalizedPath = normalizePath(filePath); - const existingNodes = normalizedModuleGraph.get(normalizedPath); - if (existingNodes) { - nodes.forEach((node) => void existingNodes.add(node)); - } else { - normalizedModuleGraph.set(normalizedPath, new Set(nodes)); - } - }); const storyIndex = await storyIndexGenerator.getIndex(); const baselineStatuses = buildIndexBaselineStatuses(storyIndex, baselineEntryIds); @@ -320,16 +526,24 @@ export class ChangeDetectionService { const statuses = new Map(); for (const changedFile of scannedFiles) { - const affectedStoryFiles = findAffectedStoryFiles( - changedFile, - normalizedModuleGraph, - storyIdsByFile - ); - const lowestDistance = Math.min( - ...Array.from(affectedStoryFiles.values(), ({ distance }) => distance) - ); + const affectedStoryFiles = reverseIndex.lookup(changedFile); + // Include the changed file as a story-at-distance-0 if it IS a story (parity with + // legacy trace-changed.ts:10-12). + const allEntries = new Map(affectedStoryFiles); + if (storyIdsByFile.has(changedFile)) { + allEntries.set(changedFile, 0); + } + if (allEntries.size === 0) { + continue; + } + let lowestDistance = Number.POSITIVE_INFINITY; + for (const distance of allEntries.values()) { + if (distance < lowestDistance) { + lowestDistance = distance; + } + } - for (const [storyFile, { distance }] of affectedStoryFiles.entries()) { + for (const [storyFile, distance] of allEntries.entries()) { const storyIds = storyIdsByFile.get(storyFile); if (!storyIds) { continue; @@ -387,6 +601,10 @@ export class ChangeDetectionService { (status) => !isSameStatus(this.previousStatuses.get(status.storyId), status) ); + if (removedStoryIds.length === 0 && changedStatuses.length === 0) { + return; + } + if (removedStoryIds.length > 0) { this.options.statusStore.unset(removedStoryIds); } @@ -411,25 +629,4 @@ export class ChangeDetectionService { this.readinessResolved = true; setChangeDetectionReadiness(readiness); } - - private handleBuilderStartupEvent( - event: Exclude - ): void { - if (event.type === 'unavailable') { - logger.warn(`Change detection unavailable: ${event.reason}`); - this.resolveReadiness({ - status: 'unavailable', - reason: event.reason, - error: event.error, - }); - } else { - logger.error(`Change detection failed: ${event.error.message}`); - this.resolveReadiness({ - status: 'error', - error: event.error, - }); - } - - void this.dispose(); - } } diff --git a/code/core/src/core-server/change-detection/GitDiffProvider.ts b/code/core/src/core-server/change-detection/GitDiffProvider.ts index 04d3cebfd319..a6326aafd187 100644 --- a/code/core/src/core-server/change-detection/GitDiffProvider.ts +++ b/code/core/src/core-server/change-detection/GitDiffProvider.ts @@ -211,6 +211,10 @@ export class GitDiffProvider { }); } + dispose(): void { + this.stopWatching(); + } + private stopWatching(): void { if (this.watchingStopped) { return; diff --git a/code/core/src/core-server/change-detection/adapters/index.ts b/code/core/src/core-server/change-detection/adapters/index.ts new file mode 100644 index 000000000000..db24f8868698 --- /dev/null +++ b/code/core/src/core-server/change-detection/adapters/index.ts @@ -0,0 +1 @@ +export type { ChangeDetectionAdapter, FileChangeEvent, ModuleResolveConfig } from './types.ts'; diff --git a/code/core/src/core-server/change-detection/adapters/types.test.ts b/code/core/src/core-server/change-detection/adapters/types.test.ts new file mode 100644 index 000000000000..8f9d3043adb0 --- /dev/null +++ b/code/core/src/core-server/change-detection/adapters/types.test.ts @@ -0,0 +1,33 @@ +// Type-level smoke tests for the ChangeDetectionAdapter contract. +import { describe, expectTypeOf, it } from 'vitest'; + +import type { ChangeDetectionAdapter, FileChangeEvent, ModuleResolveConfig } from './types.ts'; + +describe('ChangeDetectionAdapter types', () => { + it('FileChangeEvent is a discriminated union over add | change | unlink', () => { + expectTypeOf().toEqualTypeOf< + | { kind: 'add'; path: string } + | { kind: 'change'; path: string } + | { kind: 'unlink'; path: string } + >(); + }); + + it('ModuleResolveConfig.alias accepts both Record and Array<{find, replacement}>', () => { + expectTypeOf>().toMatchTypeOf< + NonNullable + >(); + expectTypeOf>().toMatchTypeOf< + NonNullable + >(); + }); + + it('ChangeDetectionAdapter.getResolveConfig returns Promise', () => { + expectTypeOf< + ChangeDetectionAdapter['getResolveConfig'] + >().returns.resolves.toEqualTypeOf(); + }); + + it('ChangeDetectionAdapter.onFileChange returns an unsubscribe function', () => { + expectTypeOf().returns.toEqualTypeOf<() => void>(); + }); +}); diff --git a/code/core/src/core-server/change-detection/adapters/types.ts b/code/core/src/core-server/change-detection/adapters/types.ts new file mode 100644 index 000000000000..0db056e5e610 --- /dev/null +++ b/code/core/src/core-server/change-detection/adapters/types.ts @@ -0,0 +1,28 @@ +/** + * Builder-agnostic change-detection adapter contract. + * + * The detector is owned by `code/core/src/core-server/change-detection/` and never imports a + * builder package. Each builder ships its own `ChangeDetectionAdapter` implementation that + * (a) supplies static resolve config once at start, and (b) pushes file-system events as + * they occur. + * + * Builder-vite is the first consumer (see `code/builders/builder-vite/src/change-detection-adapter/`). + */ + +import type { ModuleResolveConfig } from 'storybook/internal/types'; + +export type { ModuleResolveConfig }; + +export type FileChangeEvent = + | { kind: 'add'; path: string } + | { kind: 'change'; path: string } + | { kind: 'unlink'; path: string }; + +export interface ChangeDetectionAdapter { + /** Pull: builder produces resolve-config once at start; detector caches it. */ + getResolveConfig(): Promise; + /** Push: builder reports file-system events; returns an unsubscribe function. */ + onFileChange(handler: (event: FileChangeEvent) => void): () => void; + /** Optional: builder reports a startup failure so the detector can mark itself unavailable. */ + onStartupFailure?(handler: (event: { reason: string; error?: Error }) => void): () => void; +} diff --git a/code/core/src/core-server/change-detection/dependency-graph/DependencyGraphBuilder.test.ts b/code/core/src/core-server/change-detection/dependency-graph/DependencyGraphBuilder.test.ts new file mode 100644 index 000000000000..b71b153ad6fb --- /dev/null +++ b/code/core/src/core-server/change-detection/dependency-graph/DependencyGraphBuilder.test.ts @@ -0,0 +1,348 @@ +// Tests the eager forward-walk performed by DependencyGraphBuilder. +// We stub readFile + ChangeDetectionResolverFactory, and drive the walker via a real +// ParserRegistry that dispatches to an in-memory test parser — no oxc binary needed. +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { readFile } from 'node:fs/promises'; + +import { logger } from 'storybook/internal/node-logger'; + +import { ParserRegistry } from '../parser-registry/index.ts'; +import type { ImportEdge, ImportParser } from '../parser-registry/index.ts'; +import { DependencyGraphBuilder } from './DependencyGraphBuilder.ts'; +import type { ChangeDetectionResolverFactory } from './ResolverFactory.ts'; + +vi.mock('node:fs/promises', { spy: true }); +vi.mock('storybook/internal/node-logger', { spy: true }); + +interface FakeWorld { + /** Source files that exist (path -> content). Content is irrelevant — parser is stubbed. */ + files: Set; + /** Per-file outgoing edges, keyed by absolute path. */ + edges: Map; + /** Per-(from, specifier) resolutions; absent → resolver returns null. */ + resolutions: Map; +} + +/** + * Builds a real ParserRegistry with a single test parser that claims the full set of + * walkable JS/TS/MDX extensions. The parser returns per-file precomputed edges from the + * FakeWorld, standing in for oxc/mdx parsing in these unit tests. + */ +function makeFakeRegistry(world: FakeWorld): ParserRegistry { + const testParser: ImportParser = { + extensions: ['.ts', '.tsx', '.js', '.jsx', '.mjs', '.cjs', '.mdx'], + parse: vi.fn(async ({ filePath }) => world.edges.get(filePath) ?? []), + }; + return new ParserRegistry({ + defaultParsers: [testParser], + pluginParsers: [], + }); +} + +function makeFakeResolver(world: FakeWorld): ChangeDetectionResolverFactory { + return { + resolve: vi.fn(async (from: string, specifier: string) => { + const key = `${from}::${specifier}`; + return world.resolutions.has(key) ? world.resolutions.get(key)! : null; + }), + } as unknown as ChangeDetectionResolverFactory; +} + +function setupFsReadOk(world: FakeWorld) { + vi.mocked(readFile).mockImplementation(async (path) => { + if (!world.files.has(String(path))) { + throw Object.assign(new Error(`ENOENT: ${String(path)}`), { code: 'ENOENT' }); + } + return ''; + }); +} + +describe('DependencyGraphBuilder', () => { + beforeEach(() => { + vi.mocked(logger.debug).mockImplementation(() => undefined); + vi.mocked(logger.warn).mockImplementation(() => undefined); + }); + + afterEach(() => { + vi.clearAllMocks(); + }); + + it('returns empty results for an empty story list', async () => { + const world: FakeWorld = { files: new Set(), edges: new Map(), resolutions: new Map() }; + setupFsReadOk(world); + const builder = new DependencyGraphBuilder({ + registry: makeFakeRegistry(world), + resolver: makeFakeResolver(world), + workspaceRoots: new Set(), + projectRoot: '/repo', + }); + + const { reverseIndex, graph } = await builder.build([]); + + expect(reverseIndex.asMap().size).toBe(0); + expect(graph.size).toBe(0); + }); + + it('records a story with no imports at depth 0 and adds nothing else', async () => { + const story = '/repo/src/A.stories.tsx'; + const world: FakeWorld = { + files: new Set([story]), + edges: new Map([[story, []]]), + resolutions: new Map(), + }; + setupFsReadOk(world); + const builder = new DependencyGraphBuilder({ + registry: makeFakeRegistry(world), + resolver: makeFakeResolver(world), + workspaceRoots: new Set(), + projectRoot: '/repo', + }); + + const { reverseIndex } = await builder.build([story]); + + expect(reverseIndex.lookup(story).get(story)).toBe(0); + expect(reverseIndex.asMap().size).toBe(1); + }); + + it('skips CSS imports (they are not walkable)', async () => { + const story = '/repo/src/A.stories.tsx'; + const css = '/repo/src/styles.css'; + const world: FakeWorld = { + files: new Set([story, css]), + edges: new Map([[story, [{ specifier: './styles.css', kind: 'static' }]]]), + resolutions: new Map([[`${story}::./styles.css`, css]]), + }; + setupFsReadOk(world); + const builder = new DependencyGraphBuilder({ + registry: makeFakeRegistry(world), + resolver: makeFakeResolver(world), + workspaceRoots: new Set(), + projectRoot: '/repo', + }); + + const { reverseIndex } = await builder.build([story]); + + // CSS resolved into scope so the reverse index DOES record it at depth 1, but the walk + // does not recurse into it (no further calls to extractor on it). Verify only story root + // and the css leaf appear. + expect(reverseIndex.lookup(story).get(story)).toBe(0); + // CSS may be present at depth 1 (resolved into scope). Either is fine for this test — + // what matters is no transitive walk happened. + expect(reverseIndex.asMap().size).toBeLessThanOrEqual(2); + }); + + it('records sibling JS dep at depth 1 when imported by story', async () => { + const story = '/repo/src/A.stories.tsx'; + const sibling = '/repo/src/sibling.ts'; + const world: FakeWorld = { + files: new Set([story, sibling]), + edges: new Map([ + [story, [{ specifier: './sibling.ts', kind: 'static' }]], + [sibling, []], + ]), + resolutions: new Map([[`${story}::./sibling.ts`, sibling]]), + }; + setupFsReadOk(world); + const builder = new DependencyGraphBuilder({ + registry: makeFakeRegistry(world), + resolver: makeFakeResolver(world), + workspaceRoots: new Set(), + projectRoot: '/repo', + }); + + const { reverseIndex } = await builder.build([story]); + + expect(reverseIndex.lookup(sibling).get(story)).toBe(1); + }); + + it('walks into a workspace package (resolved into a workspaceRoot)', async () => { + const story = '/repo/src/A.stories.tsx'; + const wsMain = '/repo/packages/lib/src/index.ts'; + const world: FakeWorld = { + files: new Set([story, wsMain]), + edges: new Map([ + [story, [{ specifier: '@scope/lib', kind: 'static' }]], + [wsMain, []], + ]), + resolutions: new Map([[`${story}::@scope/lib`, wsMain]]), + }; + setupFsReadOk(world); + const builder = new DependencyGraphBuilder({ + registry: makeFakeRegistry(world), + resolver: makeFakeResolver(world), + workspaceRoots: new Set(['/repo/packages/lib']), + projectRoot: '/repo', + }); + + const { reverseIndex } = await builder.build([story]); + + expect(reverseIndex.lookup(wsMain).get(story)).toBe(1); + }); + + it('does NOT walk into a regular node_modules package (resolved outside scope)', async () => { + const story = '/repo/src/A.stories.tsx'; + const npmMain = '/repo/node_modules/lodash/index.js'; + const npmTransitive = '/repo/node_modules/lodash/util.js'; + const registry = makeFakeRegistry({ + files: new Set(), + edges: new Map([ + [story, [{ specifier: 'lodash', kind: 'static' }]], + [npmMain, [{ specifier: './util', kind: 'static' }]], + ]), + resolutions: new Map(), + }); + const resolver = { + resolve: vi.fn(async (from: string, spec: string) => { + if (from === story && spec === 'lodash') { + return npmMain; + } + if (from === npmMain && spec === './util') { + return npmTransitive; + } + return null; + }), + } as unknown as ChangeDetectionResolverFactory; + setupFsReadOk({ + files: new Set([story, npmMain, npmTransitive]), + edges: new Map(), + resolutions: new Map(), + }); + const builder = new DependencyGraphBuilder({ + registry, + resolver, + workspaceRoots: new Set(), + projectRoot: '/repo', + }); + + const { reverseIndex } = await builder.build([story]); + + // npmMain resolved out of scope — neither it nor its transitive dep should appear. + expect(reverseIndex.asMap().has(npmMain)).toBe(false); + expect(reverseIndex.asMap().has(npmTransitive)).toBe(false); + }); + + it('records two stories importing the same shared dep with their independent depths', async () => { + const a = '/repo/src/A.stories.tsx'; + const b = '/repo/src/B.stories.tsx'; + const intermediate = '/repo/src/intermediate.ts'; + const shared = '/repo/src/shared.ts'; + const world: FakeWorld = { + files: new Set([a, b, intermediate, shared]), + edges: new Map([ + [a, [{ specifier: './shared.ts', kind: 'static' }]], + [b, [{ specifier: './intermediate.ts', kind: 'static' }]], + [intermediate, [{ specifier: './shared.ts', kind: 'static' }]], + [shared, []], + ]), + resolutions: new Map([ + [`${a}::./shared.ts`, shared], + [`${b}::./intermediate.ts`, intermediate], + [`${intermediate}::./shared.ts`, shared], + ]), + }; + setupFsReadOk(world); + const builder = new DependencyGraphBuilder({ + registry: makeFakeRegistry(world), + resolver: makeFakeResolver(world), + workspaceRoots: new Set(), + projectRoot: '/repo', + }); + + const { reverseIndex } = await builder.build([a, b]); + + const inner = reverseIndex.lookup(shared); + expect(inner.get(a)).toBe(1); + expect(inner.get(b)).toBe(2); + }); + + it('skips one specifier that fails to resolve but records the other edges from the same file', async () => { + const story = '/repo/src/A.stories.tsx'; + const ok = '/repo/src/ok.ts'; + const world: FakeWorld = { + files: new Set([story, ok]), + edges: new Map([ + [ + story, + [ + { specifier: './missing', kind: 'static' }, + { specifier: './ok.ts', kind: 'static' }, + ], + ], + [ok, []], + ]), + // Only the ./ok.ts specifier resolves; ./missing has no entry => null. + resolutions: new Map([[`${story}::./ok.ts`, ok]]), + }; + setupFsReadOk(world); + const builder = new DependencyGraphBuilder({ + registry: makeFakeRegistry(world), + resolver: makeFakeResolver(world), + workspaceRoots: new Set(), + projectRoot: '/repo', + }); + + const { reverseIndex } = await builder.build([story]); + + expect(reverseIndex.lookup(ok).get(story)).toBe(1); + }); + + it('resolves each shared module exactly once regardless of how many stories reach it', async () => { + const storyA = '/repo/src/A.stories.tsx'; + const storyB = '/repo/src/B.stories.tsx'; + const shared = '/repo/src/shared.ts'; + const world: FakeWorld = { + files: new Set([storyA, storyB, shared]), + edges: new Map([ + [storyA, [{ specifier: './shared.ts', kind: 'static' }]], + [storyB, [{ specifier: './shared.ts', kind: 'static' }]], + [shared, [{ specifier: './nothing.ts', kind: 'static' }]], + ]), + resolutions: new Map([ + [`${storyA}::./shared.ts`, shared], + [`${storyB}::./shared.ts`, shared], + // shared's outgoing edge fails to resolve — we only care that resolver is called once + // for shared, not once per story walk that reaches it. + ]), + }; + setupFsReadOk(world); + const resolver = makeFakeResolver(world); + const builder = new DependencyGraphBuilder({ + registry: makeFakeRegistry(world), + resolver, + workspaceRoots: new Set(), + projectRoot: '/repo', + }); + + await builder.build([storyA, storyB]); + + // Without the resolve cache, `shared.ts`'s single outgoing edge would be resolved twice + // (once per story walk that visits it). With the cache, it is resolved exactly once. + const sharedEdgeResolves = vi + .mocked(resolver.resolve) + .mock.calls.filter(([from]) => from === shared); + expect(sharedEdgeResolves).toHaveLength(1); + }); + + it('emits a debug log line on completion', async () => { + const story = '/repo/src/A.stories.tsx'; + const world: FakeWorld = { + files: new Set([story]), + edges: new Map([[story, []]]), + resolutions: new Map(), + }; + setupFsReadOk(world); + const builder = new DependencyGraphBuilder({ + registry: makeFakeRegistry(world), + resolver: makeFakeResolver(world), + workspaceRoots: new Set(), + projectRoot: '/repo', + }); + + await builder.build([story]); + + expect(vi.mocked(logger.debug)).toHaveBeenCalledWith( + expect.stringContaining('Change detection graph built:') + ); + }); +}); diff --git a/code/core/src/core-server/change-detection/dependency-graph/DependencyGraphBuilder.ts b/code/core/src/core-server/change-detection/dependency-graph/DependencyGraphBuilder.ts new file mode 100644 index 000000000000..fffe2d7d4761 --- /dev/null +++ b/code/core/src/core-server/change-detection/dependency-graph/DependencyGraphBuilder.ts @@ -0,0 +1,92 @@ +import { cpus } from 'node:os'; + +import pLimit from 'p-limit'; +import { normalize } from 'pathe'; + +import { logger as defaultLogger } from 'storybook/internal/node-logger'; + +import type { ParserRegistry } from '../parser-registry/index.ts'; +import { ParseResolveCache } from './ParseResolveCache.ts'; +import { ReverseIndexImpl } from './ReverseIndex.ts'; +import type { ChangeDetectionResolverFactory } from './ResolverFactory.ts'; +import type { DependencyGraph } from './types.ts'; +import { walkFromStory } from './walkFromStory.ts'; + +interface BuilderLogger { + debug: (message: string) => void; + warn: (message: string) => void; +} + +interface BuilderOptions { + registry: ParserRegistry; + resolver: ChangeDetectionResolverFactory; + workspaceRoots: Set; + projectRoot: string; + logger?: BuilderLogger; + /** + * Optional shared cache. Pass the same instance to {@link IncrementalPatcher} so + * post-build incremental walks skip work the cold-start build already did. When + * omitted, the builder constructs a private cache that lives only for one `build()`. + */ + cache?: ParseResolveCache; +} + +/** + * Eagerly builds a {@link ReverseIndexImpl} + {@link DependencyGraph} by BFS-walking forward + * from each story file. Walks stop at workspace boundaries, non-JS extensions, or unresolved + * specifiers. Per-file errors (read failure, parse failure, individual unresolved specifiers) + * are swallowed and result in fewer recorded deps for that file — not a build failure. + * Graceful degradation ensures one bad file cannot crash change-detection for all stories. + */ +export class DependencyGraphBuilder { + private readonly registry: ParserRegistry; + private readonly logger: BuilderLogger; + private readonly cache: ParseResolveCache; + + constructor(opts: BuilderOptions) { + this.registry = opts.registry; + this.logger = opts.logger ?? defaultLogger; + this.cache = + opts.cache ?? + new ParseResolveCache({ + registry: opts.registry, + resolver: opts.resolver, + workspaceRoots: opts.workspaceRoots, + projectRoot: opts.projectRoot, + logger: this.logger, + }); + } + + async build( + storyFiles: Iterable + ): Promise<{ reverseIndex: ReverseIndexImpl; graph: DependencyGraph }> { + const startedAt = Date.now(); + const reverseIndex = new ReverseIndexImpl(); + const graph: DependencyGraph = new Map(); + + const limit = pLimit(cpus().length * 2); + + const stories = Array.from(storyFiles, (s) => normalize(s)); + + await Promise.all( + stories.map((story) => + limit(() => + walkFromStory({ + storyRoot: story, + registry: this.registry, + cache: this.cache, + reverseIndex, + recordEdges: (file, deps) => graph.set(file, deps), + }) + ) + ) + ); + + const elapsed = Date.now() - startedAt; + this.logger.debug( + `Change detection graph built: ${stories.length} stories, ${reverseIndex.asMap().size} deps tracked, ${elapsed}ms` + ); + + return { reverseIndex, graph }; + } +} diff --git a/code/core/src/core-server/change-detection/dependency-graph/IncrementalPatcher.ts b/code/core/src/core-server/change-detection/dependency-graph/IncrementalPatcher.ts new file mode 100644 index 000000000000..af00eedee8f1 --- /dev/null +++ b/code/core/src/core-server/change-detection/dependency-graph/IncrementalPatcher.ts @@ -0,0 +1,160 @@ +import { normalize } from 'pathe'; + +import { logger as defaultLogger } from 'storybook/internal/node-logger'; + +import type { FileChangeEvent } from '../adapters/types.ts'; +import type { ParserRegistry } from '../parser-registry/index.ts'; +import { ParseResolveCache } from './ParseResolveCache.ts'; +import type { ReverseIndexImpl } from './ReverseIndex.ts'; +import type { ChangeDetectionResolverFactory } from './ResolverFactory.ts'; +import type { DependencyGraph } from './types.ts'; +import { walkFromStory } from './walkFromStory.ts'; + +function setsEqual(a: Set, b: Set): boolean { + if (a.size !== b.size) { + return false; + } + for (const item of a) { + if (!b.has(item)) { + return false; + } + } + return true; +} + +interface PatcherLogger { + debug: (message: string) => void; + warn: (message: string) => void; +} + +interface PatcherOptions { + reverseIndex: ReverseIndexImpl; + graph: DependencyGraph; + registry: ParserRegistry; + resolver: ChangeDetectionResolverFactory; + workspaceRoots: Set; + projectRoot: string; + logger?: PatcherLogger; + /** Set-style predicate: returns true if the path is a story-root file. */ + isStoryFile: (path: string) => boolean; + /** + * Optional shared cache. When the patcher and {@link DependencyGraphBuilder} share the + * same instance, post-build incremental walks reuse parse + resolve results from the + * cold-start build. The patcher invalidates `path` on every `change`/`unlink` event + * before reading. + */ + cache?: ParseResolveCache; +} + +/** + * Applies a single {@link FileChangeEvent} to the live reverse index + graph. + * + * `patch()` mutates `graph` and `reverseIndex` across multiple `await` points, so the caller + * MUST serialise concurrent calls; {@link ChangeDetectionService} chains them through + * `patchQueue`. + * + * Known limitation: files that are unresolved at cold-start (resolver returns null so they + * never enter the graph) are not automatically reconnected when they later appear on disk. + * A full rebuild is required to pick them up. + */ +export class IncrementalPatcher { + private readonly reverseIndex: ReverseIndexImpl; + private readonly graph: DependencyGraph; + private readonly registry: ParserRegistry; + private readonly logger: PatcherLogger; + private readonly isStoryFile: (path: string) => boolean; + private readonly cache: ParseResolveCache; + + constructor(opts: PatcherOptions) { + this.reverseIndex = opts.reverseIndex; + this.graph = opts.graph; + this.registry = opts.registry; + this.logger = opts.logger ?? defaultLogger; + this.isStoryFile = opts.isStoryFile; + this.cache = + opts.cache ?? + new ParseResolveCache({ + registry: opts.registry, + resolver: opts.resolver, + workspaceRoots: opts.workspaceRoots, + projectRoot: opts.projectRoot, + logger: this.logger, + }); + } + + async patch(event: FileChangeEvent): Promise { + const path = normalize(event.path); + // File contents may have changed (or the file is gone); drop any stale cached + // parse/resolve data before any read. + this.cache.invalidate(path); + + if (event.kind === 'add') { + if (this.isStoryFile(path)) { + await this.walkStory(path); + } + // Non-story add: the documented limitation says we don't recover unresolved deps. + return; + } + + if (event.kind === 'unlink') { + const dependentsSet = new Set(this.reverseIndex.lookup(path).keys()); + this.graph.delete(path); + if (this.isStoryFile(path)) { + this.reverseIndex.removeStory(path); + } + // Re-walk every dependent story so transitive deps reachable only through `path` + // are pruned. + const storiesToWalk: string[] = []; + for (const story of dependentsSet) { + if (story === path || !this.isStoryFile(story)) { + continue; + } + this.reverseIndex.removeStory(story); + storiesToWalk.push(story); + } + await Promise.all(storiesToWalk.map((story) => this.walkStory(story))); + return; + } + + // 'change' on an existing file: re-walk every story that depends on this file + // (plus the file itself if it's a story). The walk re-resolves dependencies via + // the cache (already invalidated above) and rebuilds the relevant slice of the + // graph and reverse-index. + const affectedStories = new Set(this.reverseIndex.lookup(path).keys()); + if (this.isStoryFile(path)) { + affectedStories.add(path); + } + + // Skip re-walk when the file's direct dependency set is unchanged (e.g. on a + // comment-only or logic-only edit). The graph and reverse-index remain accurate. + const oldDeps = this.graph.get(path); + if (oldDeps !== undefined) { + const newDeps = await this.cache.resolveOnce(path); + if (setsEqual(oldDeps, newDeps)) { + return; + } + } + + const storiesToWalk: string[] = []; + for (const story of affectedStories) { + if (!this.isStoryFile(story)) { + continue; + } + this.reverseIndex.removeStory(story); + storiesToWalk.push(story); + } + await Promise.all(storiesToWalk.map((story) => this.walkStory(story))); + } + + private walkStory(storyRoot: string): Promise { + return walkFromStory({ + storyRoot, + registry: this.registry, + cache: this.cache, + reverseIndex: this.reverseIndex, + recordEdges: (file, deps) => { + this.graph.set(file, deps); + }, + }); + } +} diff --git a/code/core/src/core-server/change-detection/dependency-graph/ParseResolveCache.ts b/code/core/src/core-server/change-detection/dependency-graph/ParseResolveCache.ts new file mode 100644 index 000000000000..f4eb39a928e9 --- /dev/null +++ b/code/core/src/core-server/change-detection/dependency-graph/ParseResolveCache.ts @@ -0,0 +1,122 @@ +import { readFile } from 'node:fs/promises'; + +import { normalize } from 'pathe'; + +import type { ParserRegistry } from '../parser-registry/index.ts'; +import type { ImportEdge } from './types.ts'; +import type { ChangeDetectionResolverFactory } from './ResolverFactory.ts'; +import { isInScope } from './scope.ts'; + +interface CacheLogger { + debug: (message: string) => void; + warn: (message: string) => void; +} + +interface CacheOptions { + registry: ParserRegistry; + resolver: ChangeDetectionResolverFactory; + workspaceRoots: Set; + projectRoot: string; + logger: CacheLogger; +} + +/** + * Long-lived parse + resolve cache shared across cold-start build and incremental patch. + * + * Both layers were re-parsing and re-resolving every shared module on every walk before + * this cache existed (`DependencyGraphBuilder` had per-build caches; the patcher had none + * at all). Promoting the caches to instance scope and invalidating on file-change events + * collapses the redundant work without sacrificing correctness — `change`/`unlink` events + * call {@link invalidate} before the patcher reads, so a stale entry can never survive. + */ +export class ParseResolveCache { + private readonly registry: ParserRegistry; + private readonly resolver: ChangeDetectionResolverFactory; + private readonly workspaceRoots: Set; + private readonly projectRoot: string; + private readonly logger: CacheLogger; + + private readonly parseCache = new Map>(); + private readonly resolveCache = new Map>>(); + + constructor(opts: CacheOptions) { + this.registry = opts.registry; + this.resolver = opts.resolver; + this.workspaceRoots = new Set(Array.from(opts.workspaceRoots, (r) => normalize(r))); + this.projectRoot = normalize(opts.projectRoot); + this.logger = opts.logger; + } + + /** + * Parses the file once and caches the resulting edge list. Returns `[]` for unreadable + * files, parse failures, or files whose extension has no registered parser — callers + * cannot distinguish between "no edges" and "we couldn't look", which is by design: + * either way the file contributes nothing to the dependency graph. + */ + parseOnce(filePath: string): Promise { + const existing = this.parseCache.get(filePath); + if (existing) { + return existing; + } + const promise = (async (): Promise => { + let source: string; + try { + source = await readFile(filePath, 'utf8'); + } catch (error) { + this.logger.debug( + `Change detection: could not read ${filePath}: ${error instanceof Error ? error.message : String(error)}` + ); + return []; + } + try { + return (await this.registry.parse(filePath, source)) ?? []; + } catch (error) { + this.logger.debug( + `Change detection: failed to parse ${filePath}: ${error instanceof Error ? error.message : String(error)}` + ); + return []; + } + })(); + this.parseCache.set(filePath, promise); + return promise; + } + + /** Resolves every in-scope edge declared by `filePath` and returns the dep Set. */ + resolveOnce(filePath: string): Promise> { + const existing = this.resolveCache.get(filePath); + if (existing) { + return existing; + } + const promise = (async (): Promise> => { + const edges = await this.parseOnce(filePath); + const deps = new Set(); + for (const edge of edges) { + const resolved = await this.resolver.resolve(filePath, edge.specifier); + if (resolved === null) { + this.logger.debug(`Could not resolve ${edge.specifier} from ${filePath}`); + continue; + } + const normalised = normalize(resolved); + if (!isInScope(normalised, this.projectRoot, this.workspaceRoots)) { + continue; + } + deps.add(normalised); + } + return deps; + })(); + this.resolveCache.set(filePath, promise); + return promise; + } + + /** Drops both cached entries for `filePath`. Call on every `change`/`unlink` event. */ + invalidate(filePath: string): void { + this.parseCache.delete(filePath); + this.resolveCache.delete(filePath); + } + + /** Test-only: full reset. */ + clear(): void { + this.parseCache.clear(); + this.resolveCache.clear(); + } +} diff --git a/code/core/src/core-server/change-detection/dependency-graph/ResolverFactory.test.ts b/code/core/src/core-server/change-detection/dependency-graph/ResolverFactory.test.ts new file mode 100644 index 000000000000..f252f29d8e89 --- /dev/null +++ b/code/core/src/core-server/change-detection/dependency-graph/ResolverFactory.test.ts @@ -0,0 +1,248 @@ +/** + * Integration tests for ChangeDetectionResolverFactory alias and tsconfig path resolution. + * + * These tests use real file system access and the real oxc-resolver binary so that we catch + * oxc-resolver behavioural differences (e.g. trailing-slash alias keys not working, tsconfig + * path resolution) rather than just testing our own normalisation logic. + */ +import { mkdirSync, mkdtempSync, realpathSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; + +import { ChangeDetectionResolverFactory } from './ResolverFactory.ts'; + +let dir: string; + +beforeEach(() => { + // realpathSync resolves macOS /var → /private/var symlink so path comparisons work + dir = realpathSync(mkdtempSync(join(tmpdir(), 'sb-resolver-test-'))); +}); + +afterEach(() => { + rmSync(dir, { recursive: true, force: true }); +}); + +function write(relPath: string, content = 'export {};') { + const abs = join(dir, relPath); + mkdirSync(join(dir, relPath, '..'), { recursive: true }); + writeFileSync(abs, content); + return abs; +} + +function tsconfig(compilerOptions: Record, filename = 'tsconfig.json') { + const abs = join(dir, filename); + writeFileSync(abs, JSON.stringify({ compilerOptions })); + return abs; +} + +describe('ChangeDetectionResolverFactory', () => { + describe('no alias, no tsconfig', () => { + it('resolves relative imports', async () => { + const a = write('src/a.ts'); + const b = write('src/b.ts'); + const r = new ChangeDetectionResolverFactory({ projectRoot: dir }); + expect(await r.resolve(a, './b')).toBe(b); + }); + + it('returns null for bare specifiers without node_modules', async () => { + const a = write('src/a.ts'); + const r = new ChangeDetectionResolverFactory({ projectRoot: dir }); + expect(await r.resolve(a, 'react')).toBeNull(); + }); + }); + + describe('explicit alias (Record form)', () => { + it('resolves alias to absolute path', async () => { + const target = write('src/utils/index.ts'); + const from = write('src/components/Foo.ts'); + const r = new ChangeDetectionResolverFactory({ + projectRoot: dir, + alias: { utils: join(dir, 'src/utils') }, + }); + expect(await r.resolve(from, 'utils/index')).toBe(target); + }); + + it('strips trailing slash from alias key (oxc-resolver does not support @/ prefix keys)', async () => { + const target = write('src/app/Component.ts'); + const from = write('src/pages/Page.ts'); + // "@/" with trailing slash would fail in raw oxc-resolver; we normalise it to "@" + const r = new ChangeDetectionResolverFactory({ + projectRoot: dir, + alias: { '@/': join(dir, 'src') + '/' }, + }); + expect(await r.resolve(from, '@/app/Component')).toBe(target); + }); + }); + + describe('explicit alias (Array form)', () => { + it('resolves array alias with string find', async () => { + const target = write('src/utils/helper.ts'); + const from = write('src/a.ts'); + const r = new ChangeDetectionResolverFactory({ + projectRoot: dir, + alias: [{ find: 'utils', replacement: join(dir, 'src/utils') }], + }); + expect(await r.resolve(from, 'utils/helper')).toBe(target); + }); + + it('strips trailing slash from array alias find', async () => { + const target = write('src/shared/util.ts'); + const from = write('src/a.ts'); + const r = new ChangeDetectionResolverFactory({ + projectRoot: dir, + alias: [{ find: '~@/', replacement: join(dir, 'src') + '/' }], + }); + expect(await r.resolve(from, '~@/shared/util')).toBe(target); + }); + + it('returns null and does not throw for regex aliases (unsupported, opaque-leaf)', async () => { + const from = write('src/a.ts'); + write('src/utils/foo.ts'); + const r = new ChangeDetectionResolverFactory({ + projectRoot: dir, + alias: [{ find: /^utils\/(.*)/, replacement: join(dir, 'src/utils/$1') }], + }); + expect(await r.resolve(from, 'utils/foo')).toBeNull(); + }); + }); + + describe('tsconfig path aliases (auto-discovered via resolveFileAsync)', () => { + it('resolves standard wildcard pattern "@/*"', async () => { + const target = write('src/components/Button.tsx'); + const from = write('src/pages/Home.tsx'); + tsconfig({ baseUrl: '.', paths: { '@/*': ['./*'] } }); + const r = new ChangeDetectionResolverFactory({ projectRoot: dir }); + expect(await r.resolve(from, '@/src/components/Button')).toBe(target); + }); + + it('resolves named wildcard pattern "components/*"', async () => { + const target = write('src/ui/Button.tsx'); + const from = write('src/pages/Home.tsx'); + tsconfig({ baseUrl: '.', paths: { 'components/*': ['src/ui/*'] } }); + const r = new ChangeDetectionResolverFactory({ projectRoot: dir }); + expect(await r.resolve(from, 'components/Button')).toBe(target); + }); + + it('resolves multiple tsconfig path entries', async () => { + const btn = write('src/ui/Button.tsx'); + const util = write('src/helpers/format.ts'); + const from = write('src/pages/Home.tsx'); + tsconfig({ + baseUrl: '.', + paths: { + 'components/*': ['src/ui/*'], + 'helpers/*': ['src/helpers/*'], + }, + }); + const r = new ChangeDetectionResolverFactory({ projectRoot: dir }); + expect(await r.resolve(from, 'components/Button')).toBe(btn); + expect(await r.resolve(from, 'helpers/format')).toBe(util); + }); + + it('explicit alias and tsconfig wildcard paths resolve consistently', async () => { + const target = write('src/ui/Button.tsx'); + const from = write('src/pages/Home.tsx'); + tsconfig({ baseUrl: '.', paths: { '@/*': ['src/ui/*'] } }); + const r = new ChangeDetectionResolverFactory({ projectRoot: dir }); + expect(await r.resolve(from, '@/Button')).toBe(target); + }); + + it('returns null for tsconfig path alias when no tsconfig exists', async () => { + const from = write('src/a.ts'); + // No tsconfig.json written — @/* has no resolver backing + const r = new ChangeDetectionResolverFactory({ projectRoot: dir }); + expect(await r.resolve(from, '@/src/a')).toBeNull(); + }); + + it('handles tsconfig without baseUrl (paths relative to tsconfig dir)', async () => { + const target = write('src/components/Button.tsx'); + const from = write('src/pages/Home.tsx'); + // No baseUrl — TypeScript resolves paths relative to tsconfig dir + tsconfig({ paths: { '@/*': ['src/*'] } }); + const r = new ChangeDetectionResolverFactory({ projectRoot: dir }); + expect(await r.resolve(from, '@/components/Button')).toBe(target); + }); + + it('resolves "@/*": ["./*"] from deeply nested source file (dify-plus pattern)', async () => { + // Reproduces: @/app/components/base/icons/IconBase unresolved from + // app/components/base/icons/src/vender/line/others/Tools.tsx + // oxc-resolver's resolveFileAsync walks up from the file to find tsconfig.json + const target = write('app/components/base/icons/IconBase.tsx'); + const from = write('app/components/base/icons/src/vender/line/others/Tools.tsx'); + tsconfig({ paths: { '@/*': ['./*'], '~@/*': ['./*'] } }); + const r = new ChangeDetectionResolverFactory({ projectRoot: dir }); + expect(await r.resolve(from, '@/app/components/base/icons/IconBase')).toBe(target); + }); + }); + + /** + * Yarn-workspace cross-package resolution + * + * In a monorepo the root `tsconfig.json` typically maps workspace package names to their + * source directories (e.g. "@mantinex/*" -> ["./packages/@mantinex/ * /src"]). Per-package + * tsconfigs often do NOT extend the root, so `tsconfig: 'auto'` only sees the nearest + * (package-level) tsconfig and misses these workspace paths -> NotFound("@mantinex/demo"). + * + * The fix in ResolverFactory uses the root tsconfig explicitly when it exists, so that + * workspace-level path mappings are always consulted. + */ + describe('yarn workspace cross-package resolution', () => { + it('resolves workspace sibling via root tsconfig paths when package-level tsconfig has no mapping', async () => { + // Reproduces: @mantinex/demo unresolved from packages/@docs/demos/src/.../YearPicker/ + // Root tsconfig has "@mantinex/*" paths; package tsconfig has no paths and no extends. + const target = write('packages/@mantinex/demo/src/index.ts'); + // Package-level tsconfig — no paths, no extends (mirrors real mantine setup) + mkdirSync(join(dir, 'packages/@docs/demos'), { recursive: true }); + writeFileSync( + join(dir, 'packages/@docs/demos/tsconfig.json'), + JSON.stringify({ compilerOptions: {} }) + ); + // Root tsconfig — maps workspace packages to source directories + tsconfig({ paths: { '@mantinex/*': ['./packages/@mantinex/*/src'] } }); + const from = write( + 'packages/@docs/demos/src/demos/dates/YearPicker/YearPicker.demo.controlProps.tsx' + ); + + const r = new ChangeDetectionResolverFactory({ projectRoot: dir }); + expect(await r.resolve(from, '@mantinex/demo')).toBe(target); + }); + + it('resolves workspace sibling via node_modules symlink (yarn workspaces classic linker)', async () => { + // yarn workspace classic linker symlinks workspace packages at root node_modules. + // The resolver must traverse up past sub-package boundaries to reach root node_modules. + const target = write('packages/@mantinex/demo/src/index.ts'); + writeFileSync( + join(dir, 'packages/@mantinex/demo/package.json'), + JSON.stringify({ name: '@mantinex/demo', main: './src/index.ts' }) + ); + const from = write('packages/@docs/demos/src/demos/dates/YearPicker/file.tsx'); + + mkdirSync(join(dir, 'node_modules/@mantinex'), { recursive: true }); + symlinkSync( + join(dir, 'packages/@mantinex/demo'), + join(dir, 'node_modules/@mantinex/demo'), + 'dir' + ); + + const r = new ChangeDetectionResolverFactory({ projectRoot: dir }); + expect(await r.resolve(from, '@mantinex/demo')).toBe(target); + }); + + it('resolves multiple workspace siblings from the same root tsconfig paths', async () => { + const demoTarget = write('packages/@mantinex/demo/src/index.ts'); + const iconsTarget = write('packages/@mantinex/dev-icons/src/index.ts'); + tsconfig({ + paths: { + '@mantinex/*': ['./packages/@mantinex/*/src'], + }, + }); + const from = write('packages/@docs/demos/src/file.tsx'); + + const r = new ChangeDetectionResolverFactory({ projectRoot: dir }); + expect(await r.resolve(from, '@mantinex/demo')).toBe(demoTarget); + expect(await r.resolve(from, '@mantinex/dev-icons')).toBe(iconsTarget); + }); + }); +}); diff --git a/code/core/src/core-server/change-detection/dependency-graph/ResolverFactory.ts b/code/core/src/core-server/change-detection/dependency-graph/ResolverFactory.ts new file mode 100644 index 000000000000..250c30611f47 --- /dev/null +++ b/code/core/src/core-server/change-detection/dependency-graph/ResolverFactory.ts @@ -0,0 +1,159 @@ +import { join } from 'node:path'; + +import { ResolverFactory as OxcResolverFactory } from 'oxc-resolver'; + +import { logger } from 'storybook/internal/node-logger'; + +import type { ModuleResolveConfig } from '../adapters/types.ts'; + +const DEFAULT_EXTENSIONS = ['.tsx', '.ts', '.d.ts', '.jsx', '.js', '.mjs', '.cjs', '.json']; +const DEFAULT_CONDITIONS = ['storybook', 'import', 'module', 'default']; + +/** + * `ModuleResolveConfig.alias` accepts both Vite shapes: + * - `Record` (object form) + * - `Array<{ find: string | RegExp; replacement: string }>` (array form) + * + * `oxc-resolver` expects `Record>`. + * RegExp `find` entries cannot be expressed in oxc-resolver's alias config and + * are skipped with a debug log (downgraded to opaque-leaf at resolve time). + * + * Instance-scoped so each ChangeDetectionResolverFactory warns independently — + * multiple service instances in the same process do not suppress each other's warnings. + */ +class AliasNormalizer { + private readonly warnedRegexAliases = new Set(); + + normalize( + alias: ModuleResolveConfig['alias'] + ): Record> | undefined { + if (!alias) { + return undefined; + } + + const out: Record> = {}; + const skippedRegex: string[] = []; + + if (Array.isArray(alias)) { + for (const entry of alias) { + if (typeof entry.find === 'string') { + const find = entry.find.replace(/\/$/, ''); + const replacement = entry.replacement.replace(/\/$/, ''); + out[find] = [replacement]; + } else { + skippedRegex.push(String(entry.find)); + } + } + } else { + for (const [find, replacement] of Object.entries(alias)) { + out[find.replace(/\/$/, '')] = [replacement.replace(/\/$/, '')]; + } + } + + if (skippedRegex.length > 0) { + const newPatterns = skippedRegex.filter((p) => !this.warnedRegexAliases.has(p)); + if (newPatterns.length > 0) { + for (const p of newPatterns) { + this.warnedRegexAliases.add(p); + } + logger.warn( + `Change detection: ignored ${skippedRegex.length} regex alias(es); related modules tracked as opaque-leaf.` + ); + logger.debug( + `ChangeDetectionResolverFactory: skipped regex aliases [${skippedRegex.join(', ')}]` + ); + } else { + for (const pattern of skippedRegex) { + logger.debug(`ChangeDetectionResolverFactory: skipping regex alias '${pattern}'`); + } + } + } + + return Object.keys(out).length > 0 ? out : undefined; + } +} + +/** + * Thin wrapper around `oxc-resolver`'s `ResolverFactory` configured per a + * builder-supplied {@link ModuleResolveConfig}. Normalizes the alias map shape and + * converts resolver errors to `null` (with a debug log) — the caller treats + * unresolvable specifiers as opaque-leaf edges. + */ +export class ChangeDetectionResolverFactory { + private readonly factory: OxcResolverFactory; + private readonly aliasNormalizer = new AliasNormalizer(); + /** + * Virtual entry point placed directly in the project root. + * + * Used as the fallback `from` path when the primary resolution fails. + * Resolving from here guarantees that `resolveFileAsync`'s tsconfig + * walk-up reaches the root `tsconfig.json` before any intermediate + * per-package tsconfig — so workspace-level `paths` mappings are + * consulted even when the per-package tsconfig does not extend root. + */ + private readonly projectRootEntry: string; + + constructor(config: ModuleResolveConfig) { + const alias = this.aliasNormalizer.normalize(config.alias); + const conditionNames = config.conditions ?? DEFAULT_CONDITIONS; + + this.factory = new OxcResolverFactory({ + tsconfig: 'auto', + alias, + conditionNames, + extensions: DEFAULT_EXTENSIONS, + }); + + this.projectRootEntry = join(config.projectRoot, '__sb_resolver_root__.ts'); + } + + /** + * Resolves `specifier` from the file at `from` (must be an absolute path). + * + * Two-pass strategy: + * 1. Resolve from `from` — handles per-package tsconfig paths and local node_modules. + * 2. On failure, retry from the project root — picks up root-level tsconfig `paths` + * (e.g. workspace package aliases) that intermediate per-package tsconfigs may + * not inherit, as well as root-level node_modules symlinks. + * + * Returns the absolute resolved path, or `null` if both passes fail. + * Never throws — internal errors are converted to `null` and a debug-level log + * line is emitted. + */ + async resolve(from: string, specifier: string): Promise { + try { + const result = await this.factory.resolveFileAsync(from, specifier); + if (result.path) { + return result.path; + } + + // Fallback: retry from the project root so that root-level tsconfig paths + // and root node_modules are consulted. Skip the fallback when `from` is + // already the virtual root entry to avoid a redundant second attempt. + if (from !== this.projectRootEntry) { + const rootResult = await this.factory.resolveFileAsync(this.projectRootEntry, specifier); + if (rootResult.path) { + return rootResult.path; + } + if (result.error ?? rootResult.error) { + logger.debug( + `ChangeDetectionResolverFactory: '${specifier}' from '${from}' unresolved (${result.error ?? rootResult.error})` + ); + } + return null; + } + + if (result.error) { + logger.debug( + `ChangeDetectionResolverFactory: '${specifier}' from '${from}' unresolved (${result.error})` + ); + } + return null; + } catch (error) { + logger.debug( + `ChangeDetectionResolverFactory: error resolving '${specifier}' from '${from}': ${String(error)}` + ); + return null; + } + } +} diff --git a/code/core/src/core-server/change-detection/dependency-graph/ReverseIndex.test.ts b/code/core/src/core-server/change-detection/dependency-graph/ReverseIndex.test.ts new file mode 100644 index 000000000000..abf96dcc67e2 --- /dev/null +++ b/code/core/src/core-server/change-detection/dependency-graph/ReverseIndex.test.ts @@ -0,0 +1,98 @@ +// Covers the construction + mutation surface of ReverseIndexImpl. +import { describe, expect, it } from 'vitest'; + +import { ReverseIndexImpl } from './ReverseIndex.ts'; + +describe('ReverseIndexImpl', () => { + it('returns an empty Map (not undefined) when looking up an unknown dep', () => { + const index = new ReverseIndexImpl(); + + const result = index.lookup('/repo/src/foo.ts'); + + expect(result).toBeInstanceOf(Map); + expect(result.size).toBe(0); + }); + + it('records (dep, story, depth) and surfaces the depth via lookup', () => { + const index = new ReverseIndexImpl(); + + index.record('/repo/src/foo.ts', '/repo/src/Foo.stories.tsx', 1); + + const result = index.lookup('/repo/src/foo.ts'); + expect(result.get('/repo/src/Foo.stories.tsx')).toBe(1); + }); + + it('keeps min(existing, depth) when the same (dep, story) is recorded twice with different depths', () => { + const index = new ReverseIndexImpl(); + + index.record('/repo/src/foo.ts', '/repo/src/A.stories.tsx', 3); + index.record('/repo/src/foo.ts', '/repo/src/A.stories.tsx', 1); + index.record('/repo/src/foo.ts', '/repo/src/A.stories.tsx', 2); + + expect(index.lookup('/repo/src/foo.ts').get('/repo/src/A.stories.tsx')).toBe(1); + }); + + it('records the same dep against multiple stories and surfaces both via lookup', () => { + const index = new ReverseIndexImpl(); + + index.record('/repo/src/shared.ts', '/repo/src/A.stories.tsx', 1); + index.record('/repo/src/shared.ts', '/repo/src/B.stories.tsx', 2); + + const result = index.lookup('/repo/src/shared.ts'); + expect(result.size).toBe(2); + expect(result.get('/repo/src/A.stories.tsx')).toBe(1); + expect(result.get('/repo/src/B.stories.tsx')).toBe(2); + }); + + it('removeStory deletes the story from every inner map and prunes outer entries that become empty', () => { + const index = new ReverseIndexImpl(); + index.record('/repo/src/x.ts', '/repo/src/A.stories.tsx', 1); + index.record('/repo/src/x.ts', '/repo/src/B.stories.tsx', 2); + index.record('/repo/src/y.ts', '/repo/src/A.stories.tsx', 1); + + index.removeStory('/repo/src/A.stories.tsx'); + + // /repo/src/y.ts had only A — should be pruned. + expect(index.asMap().has('/repo/src/y.ts')).toBe(false); + // /repo/src/x.ts retained for B; A removed. + const inner = index.lookup('/repo/src/x.ts'); + expect(inner.size).toBe(1); + expect(inner.get('/repo/src/B.stories.tsx')).toBe(2); + expect(inner.has('/repo/src/A.stories.tsx')).toBe(false); + }); + + it('removeEdge removes only the (dep, story) pair without touching other stories', () => { + const index = new ReverseIndexImpl(); + index.record('/repo/src/x.ts', '/repo/src/A.stories.tsx', 1); + index.record('/repo/src/x.ts', '/repo/src/B.stories.tsx', 2); + + index.removeEdge('/repo/src/x.ts', '/repo/src/A.stories.tsx'); + + const inner = index.lookup('/repo/src/x.ts'); + expect(inner.has('/repo/src/A.stories.tsx')).toBe(false); + expect(inner.get('/repo/src/B.stories.tsx')).toBe(2); + }); + + it('removeEdge prunes the outer entry when it becomes empty', () => { + const index = new ReverseIndexImpl(); + index.record('/repo/src/x.ts', '/repo/src/A.stories.tsx', 1); + + index.removeEdge('/repo/src/x.ts', '/repo/src/A.stories.tsx'); + + expect(index.asMap().has('/repo/src/x.ts')).toBe(false); + }); + + it('handles cycles by retaining the minimum depth across multiple recordings', () => { + // A imports B (depth 1); B imports C (depth 2); C imports B (would be depth 3). + // Whatever order build() visits in, recording min keeps depth 1 for B and 2 for C from story A. + const index = new ReverseIndexImpl(); + index.record('/repo/src/B.ts', '/repo/src/A.stories.tsx', 1); + index.record('/repo/src/C.ts', '/repo/src/A.stories.tsx', 2); + // Cycle re-entry attempts: + index.record('/repo/src/B.ts', '/repo/src/A.stories.tsx', 3); + index.record('/repo/src/C.ts', '/repo/src/A.stories.tsx', 4); + + expect(index.lookup('/repo/src/B.ts').get('/repo/src/A.stories.tsx')).toBe(1); + expect(index.lookup('/repo/src/C.ts').get('/repo/src/A.stories.tsx')).toBe(2); + }); +}); diff --git a/code/core/src/core-server/change-detection/dependency-graph/ReverseIndex.ts b/code/core/src/core-server/change-detection/dependency-graph/ReverseIndex.ts new file mode 100644 index 000000000000..732e0b2608f0 --- /dev/null +++ b/code/core/src/core-server/change-detection/dependency-graph/ReverseIndex.ts @@ -0,0 +1,82 @@ +import type { ReverseIndex } from './types.ts'; + +/** + * In-memory reverse index from dep file → story file → shortest BFS depth. + * + * **Contract:** callers MUST pass already-normalised paths (`pathe.normalize`); the index + * does not re-normalise on each mutation. Producers in `DependencyGraphBuilder` and + * `IncrementalPatcher` already normalise at the boundary. + */ +export class ReverseIndexImpl { + private readonly index: ReverseIndex = new Map(); + /** Forward mapping from story file -> Set of dep files it reaches. */ + private readonly forwardIndex = new Map>(); + + /** Records (or updates with min) the depth for (dep, story). */ + record(dep: string, story: string, depth: number): void { + let inner = this.index.get(dep); + if (!inner) { + inner = new Map(); + this.index.set(dep, inner); + } + const previous = inner.get(story); + if (previous === undefined || depth < previous) { + inner.set(story, depth); + + let deps = this.forwardIndex.get(story); + if (!deps) { + deps = new Set(); + this.forwardIndex.set(story, deps); + } + deps.add(dep); + } + } + + /** Removes a story from every inner map; prunes outer entries that become empty. */ + removeStory(story: string): void { + const deps = this.forwardIndex.get(story); + if (!deps) { + return; + } + for (const dep of deps) { + const inner = this.index.get(dep); + if (inner) { + inner.delete(story); + if (inner.size === 0) { + this.index.delete(dep); + } + } + } + this.forwardIndex.delete(story); + } + + /** Removes a single (dep, story) pair without affecting other stories' depths to that dep. */ + removeEdge(dep: string, story: string): void { + const inner = this.index.get(dep); + if (!inner) { + return; + } + if (inner.delete(story)) { + if (inner.size === 0) { + this.index.delete(dep); + } + const deps = this.forwardIndex.get(story); + if (deps) { + deps.delete(dep); + if (deps.size === 0) { + this.forwardIndex.delete(story); + } + } + } + } + + /** Returns the per-story depth map for dep. EMPTY map (not undefined) if dep unknown. */ + lookup(dep: string): Map { + return this.index.get(dep) ?? new Map(); + } + + /** Internal state inspection — for tests. */ + asMap(): ReverseIndex { + return this.index; + } +} diff --git a/code/core/src/core-server/change-detection/dependency-graph/incremental-patch.test.ts b/code/core/src/core-server/change-detection/dependency-graph/incremental-patch.test.ts new file mode 100644 index 000000000000..1c4d3ea41b06 --- /dev/null +++ b/code/core/src/core-server/change-detection/dependency-graph/incremental-patch.test.ts @@ -0,0 +1,364 @@ +// Covers the IncrementalPatcher behaviour for add/change/unlink events. +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { readFile } from 'node:fs/promises'; + +import { logger } from 'storybook/internal/node-logger'; + +import { ParserRegistry } from '../parser-registry/index.ts'; +import type { ImportEdge, ImportParser } from '../parser-registry/index.ts'; +import { IncrementalPatcher } from './IncrementalPatcher.ts'; +import type { ChangeDetectionResolverFactory } from './ResolverFactory.ts'; +import { ReverseIndexImpl } from './ReverseIndex.ts'; +import type { DependencyGraph } from './types.ts'; + +vi.mock('node:fs/promises', { spy: true }); +vi.mock('storybook/internal/node-logger', { spy: true }); + +interface PatcherWorld { + edges: Map; + resolutions: Map; +} + +interface TestRegistry { + registry: ParserRegistry; + parseSpy: ReturnType; +} + +/** + * Build a real ParserRegistry with a single test parser that reads precomputed edges out + * of the PatcherWorld. Exposes the parse spy so tests can assert how many times a path + * was dispatched to the parser. + */ +function makeRegistry(world: PatcherWorld): TestRegistry { + const parseSpy = vi.fn(async ({ filePath }: { filePath: string }) => { + return world.edges.get(filePath) ?? []; + }); + const testParser: ImportParser = { + extensions: ['.ts', '.tsx', '.js', '.jsx', '.mjs', '.cjs', '.mdx'], + parse: parseSpy, + }; + const registry = new ParserRegistry({ + defaultParsers: [testParser], + pluginParsers: [], + }); + return { registry, parseSpy }; +} + +function makeResolver(world: PatcherWorld): ChangeDetectionResolverFactory { + return { + resolve: vi.fn(async (from: string, specifier: string) => { + const key = `${from}::${specifier}`; + return world.resolutions.has(key) ? world.resolutions.get(key)! : null; + }), + } as unknown as ChangeDetectionResolverFactory; +} + +function setupReadFile() { + vi.mocked(readFile).mockImplementation(async () => ''); +} + +function buildPatcher(opts: { + reverseIndex?: ReverseIndexImpl; + graph?: DependencyGraph; + storyFiles?: Set; + world: PatcherWorld; +}): { + patcher: IncrementalPatcher; + reverseIndex: ReverseIndexImpl; + graph: DependencyGraph; + parseSpy: ReturnType; +} { + const reverseIndex = opts.reverseIndex ?? new ReverseIndexImpl(); + const graph = opts.graph ?? new Map(); + const storyFiles = opts.storyFiles ?? new Set(); + const { registry, parseSpy } = makeRegistry(opts.world); + const resolver = makeResolver(opts.world); + const patcher = new IncrementalPatcher({ + reverseIndex, + graph, + registry, + resolver, + workspaceRoots: new Set(), + projectRoot: '/repo', + isStoryFile: (path: string) => storyFiles.has(path), + }); + return { patcher, reverseIndex, graph, parseSpy }; +} + +describe('IncrementalPatcher', () => { + beforeEach(() => { + setupReadFile(); + vi.mocked(logger.debug).mockImplementation(() => undefined); + vi.mocked(logger.warn).mockImplementation(() => undefined); + }); + + afterEach(() => { + vi.clearAllMocks(); + }); + + it('treats a `change` event for an unknown non-story file as a no-op', async () => { + const { patcher, reverseIndex, graph, parseSpy } = buildPatcher({ + world: { edges: new Map(), resolutions: new Map() }, + }); + + await patcher.patch({ kind: 'change', path: '/repo/src/unknown.ts' }); + + // No dependents in reverseIndex, not a story file — nothing to re-walk. + expect(reverseIndex.asMap().size).toBe(0); + expect(parseSpy).not.toHaveBeenCalled(); + expect(graph.has('/repo/src/unknown.ts')).toBe(false); + }); + + it('re-walks a story root on `change`, updating depths', async () => { + const story = '/repo/src/A.stories.tsx'; + const dep = '/repo/src/dep.ts'; + const world: PatcherWorld = { + edges: new Map([ + [story, [{ specifier: './dep.ts', kind: 'static' }]], + [dep, []], + ]), + resolutions: new Map([[`${story}::./dep.ts`, dep]]), + }; + const initialIndex = new ReverseIndexImpl(); + initialIndex.record(story, story, 0); + const { patcher, reverseIndex } = buildPatcher({ + world, + reverseIndex: initialIndex, + storyFiles: new Set([story]), + }); + + await patcher.patch({ kind: 'change', path: story }); + + expect(reverseIndex.lookup(dep).get(story)).toBe(1); + }); + + it('removes (oldDep, story) edges when an import disappears on `change`', async () => { + const story = '/repo/src/A.stories.tsx'; + const oldDep = '/repo/src/old.ts'; + const newDep = '/repo/src/new.ts'; + const initialIndex = new ReverseIndexImpl(); + initialIndex.record(story, story, 0); + initialIndex.record(oldDep, story, 1); + const initialGraph: DependencyGraph = new Map([[story, new Set([oldDep])]]); + const world: PatcherWorld = { + edges: new Map([ + [story, [{ specifier: './new.ts', kind: 'static' }]], + [newDep, []], + ]), + resolutions: new Map([[`${story}::./new.ts`, newDep]]), + }; + const { patcher, reverseIndex } = buildPatcher({ + world, + reverseIndex: initialIndex, + graph: initialGraph, + storyFiles: new Set([story]), + }); + + await patcher.patch({ kind: 'change', path: story }); + + expect(reverseIndex.asMap().has(oldDep)).toBe(false); + expect(reverseIndex.lookup(newDep).get(story)).toBe(1); + }); + + it('adds NEW edges with the correct depth on `change`', async () => { + const story = '/repo/src/A.stories.tsx'; + const newDep = '/repo/src/added.ts'; + const initialIndex = new ReverseIndexImpl(); + initialIndex.record(story, story, 0); + const initialGraph: DependencyGraph = new Map([[story, new Set()]]); + const world: PatcherWorld = { + edges: new Map([ + [story, [{ specifier: './added.ts', kind: 'static' }]], + [newDep, []], + ]), + resolutions: new Map([[`${story}::./added.ts`, newDep]]), + }; + const { patcher, reverseIndex } = buildPatcher({ + world, + reverseIndex: initialIndex, + graph: initialGraph, + storyFiles: new Set([story]), + }); + + await patcher.patch({ kind: 'change', path: story }); + + expect(reverseIndex.lookup(newDep).get(story)).toBe(1); + }); + + it('removes a story root on `unlink` and clears its reverse-index contribution', async () => { + const story = '/repo/src/A.stories.tsx'; + const dep = '/repo/src/dep.ts'; + const initialIndex = new ReverseIndexImpl(); + initialIndex.record(story, story, 0); + initialIndex.record(dep, story, 1); + const initialGraph: DependencyGraph = new Map([[story, new Set([dep])]]); + const { patcher, reverseIndex, graph } = buildPatcher({ + world: { edges: new Map(), resolutions: new Map() }, + reverseIndex: initialIndex, + graph: initialGraph, + storyFiles: new Set([story]), + }); + + await patcher.patch({ kind: 'unlink', path: story }); + + expect(reverseIndex.asMap().has(dep)).toBe(false); + expect(reverseIndex.asMap().has(story)).toBe(false); + expect(graph.has(story)).toBe(false); + }); + + it('on `unlink` of a non-story dep, re-walks every story that previously reached it', async () => { + const story = '/repo/src/A.stories.tsx'; + const dep = '/repo/src/dep.ts'; + const initialIndex = new ReverseIndexImpl(); + initialIndex.record(story, story, 0); + initialIndex.record(dep, story, 1); + const initialGraph: DependencyGraph = new Map([ + [story, new Set([dep])], + [dep, new Set()], + ]); + const world: PatcherWorld = { + edges: new Map([[story, []]]), + resolutions: new Map(), + }; + const { patcher, reverseIndex } = buildPatcher({ + world, + reverseIndex: initialIndex, + graph: initialGraph, + storyFiles: new Set([story]), + }); + + await patcher.patch({ kind: 'unlink', path: dep }); + + // dep removed from index; story re-walked with no deps so only story@0 remains. + expect(reverseIndex.asMap().has(dep)).toBe(false); + expect(reverseIndex.lookup(story).get(story)).toBe(0); + }); + + it('runs full BFS for an `add` event for a new story root', async () => { + const story = '/repo/src/New.stories.tsx'; + const dep = '/repo/src/dep.ts'; + const world: PatcherWorld = { + edges: new Map([ + [story, [{ specifier: './dep.ts', kind: 'static' }]], + [dep, []], + ]), + resolutions: new Map([[`${story}::./dep.ts`, dep]]), + }; + const { patcher, reverseIndex } = buildPatcher({ + world, + storyFiles: new Set([story]), + }); + + await patcher.patch({ kind: 'add', path: story }); + + expect(reverseIndex.lookup(story).get(story)).toBe(0); + expect(reverseIndex.lookup(dep).get(story)).toBe(1); + }); + + it('skips re-walk on `change` when dep set is unchanged (comment-only edit)', async () => { + const story = '/repo/src/A.stories.tsx'; + const dep = '/repo/src/shared.ts'; + + const initialIndex = new ReverseIndexImpl(); + initialIndex.record(story, story, 0); + initialIndex.record(dep, story, 1); + const initialGraph: DependencyGraph = new Map([ + [story, new Set([dep])], + [dep, new Set()], + ]); + + // dep changes but its import list stays empty (comment-only edit) + const world: PatcherWorld = { + edges: new Map([[dep, []]]), + resolutions: new Map(), + }; + const { patcher, reverseIndex, parseSpy } = buildPatcher({ + world, + reverseIndex: initialIndex, + graph: initialGraph, + storyFiles: new Set([story]), + }); + + await patcher.patch({ kind: 'change', path: dep }); + + // story must NOT be re-walked — graph and index are still accurate + const storyCalls = parseSpy.mock.calls.filter((c) => c[0].filePath === story); + expect(storyCalls.length).toBe(0); + // reverse-index entries are preserved unchanged + expect(reverseIndex.lookup(dep).get(story)).toBe(1); + expect(reverseIndex.lookup(story).get(story)).toBe(0); + }); + + it('treats `add` for a non-story file as a no-op', async () => { + const file = '/repo/src/loose.ts'; + const world: PatcherWorld = { edges: new Map(), resolutions: new Map() }; + const { patcher, reverseIndex, parseSpy } = buildPatcher({ + world, + storyFiles: new Set(), + }); + + await patcher.patch({ kind: 'add', path: file }); + + expect(reverseIndex.asMap().size).toBe(0); + expect(parseSpy).not.toHaveBeenCalled(); + }); + + it('parses the story root exactly once when the graph entry is absent (no diff-check path)', async () => { + // graph is empty so graph.get(path) === undefined → skip the resolveOnce diff-check + // → walkStory calls resolveOnce once → parseOnce called exactly once. + const story = '/repo/src/A.stories.tsx'; + const initialIndex = new ReverseIndexImpl(); + initialIndex.record(story, story, 0); + const world: PatcherWorld = { + edges: new Map([[story, []]]), + resolutions: new Map(), + }; + const { patcher, parseSpy } = buildPatcher({ + world, + reverseIndex: initialIndex, + storyFiles: new Set([story]), + }); + + await patcher.patch({ kind: 'change', path: story }); + + const calls = parseSpy.mock.calls.filter((c) => c[0].filePath === story); + expect(calls.length).toBe(1); + }); + + it('unlink of a dep clears the dep from the graph and from the story reverse-index, not just from the index edges', async () => { + // Models the stale-cache regression: story → dep is seeded into graph + reverseIndex at + // cold-start. Without importer invalidation on unlink, walkFromStory re-uses the cached + // resolveCache entry for `story` (which still lists `dep`), silently re-adding dep to + // the graph and leaving reverseIndex.lookup(dep) non-empty. + const story = '/repo/src/A.stories.tsx'; + const dep = '/repo/src/dep.ts'; + + const initialIndex = new ReverseIndexImpl(); + initialIndex.record(story, story, 0); + initialIndex.record(dep, story, 1); + const initialGraph: DependencyGraph = new Map([ + [story, new Set([dep])], + [dep, new Set()], + ]); + + // After unlink, the story no longer imports dep. + const world: PatcherWorld = { + edges: new Map([[story, []]]), + resolutions: new Map(), + }; + const { patcher, reverseIndex, graph } = buildPatcher({ + world, + reverseIndex: initialIndex, + graph: initialGraph, + storyFiles: new Set([story]), + }); + + await patcher.patch({ kind: 'unlink', path: dep }); + + expect(reverseIndex.lookup(dep).size).toBe(0); + expect(graph.has(dep)).toBe(false); + // The story itself still exists at depth 0; dep is no longer reachable from it. + expect(reverseIndex.lookup(story).get(story)).toBe(0); + }); +}); diff --git a/code/core/src/core-server/change-detection/dependency-graph/index.ts b/code/core/src/core-server/change-detection/dependency-graph/index.ts new file mode 100644 index 000000000000..e0ddc7c77d53 --- /dev/null +++ b/code/core/src/core-server/change-detection/dependency-graph/index.ts @@ -0,0 +1,6 @@ +export type { ImportEdge, ReverseIndex, DependencyGraph } from './types.ts'; +export { ChangeDetectionResolverFactory } from './ResolverFactory.ts'; +export { ReverseIndexImpl } from './ReverseIndex.ts'; +export { DependencyGraphBuilder } from './DependencyGraphBuilder.ts'; +export { IncrementalPatcher } from './IncrementalPatcher.ts'; +export { ParseResolveCache } from './ParseResolveCache.ts'; diff --git a/code/core/src/core-server/change-detection/dependency-graph/scope.ts b/code/core/src/core-server/change-detection/dependency-graph/scope.ts new file mode 100644 index 000000000000..c59c0660f19b --- /dev/null +++ b/code/core/src/core-server/change-detection/dependency-graph/scope.ts @@ -0,0 +1,28 @@ +const NODE_MODULES_SEGMENT = '/node_modules/'; + +export function isInsideAnyWorkspace(absolute: string, workspaceRoots: Set): boolean { + if (absolute.includes(NODE_MODULES_SEGMENT)) { + return false; + } + for (const root of workspaceRoots) { + if (absolute === root || absolute.startsWith(root.endsWith('/') ? root : `${root}/`)) { + return true; + } + } + return false; +} + +export function isInScope( + absolute: string, + projectRoot: string, + workspaceRoots: Set +): boolean { + const projectPrefix = projectRoot.endsWith('/') ? projectRoot : `${projectRoot}/`; + if ( + (absolute === projectRoot || absolute.startsWith(projectPrefix)) && + !absolute.includes(NODE_MODULES_SEGMENT) + ) { + return true; + } + return isInsideAnyWorkspace(absolute, workspaceRoots); +} diff --git a/code/core/src/core-server/change-detection/dependency-graph/types.ts b/code/core/src/core-server/change-detection/dependency-graph/types.ts new file mode 100644 index 000000000000..736b8ad93d84 --- /dev/null +++ b/code/core/src/core-server/change-detection/dependency-graph/types.ts @@ -0,0 +1,15 @@ +import type { ImportEdge } from '../parser-registry/index.ts'; + +export type { ImportEdge }; + +/** + * Reverse index from dep file path → story file → shortest forward-walk depth from that story. + * Inner number preserves the BFS hop-count semantics used by `ChangeDetectionService.buildStatuses` + * to distinguish `modified` (closest stories) from `affected` (farther stories). + * + * Keys are absolute paths normalised via `pathe.normalize`. + */ +export type ReverseIndex = Map>; + +/** Per-file outgoing edges, used by IncrementalPatcher to compute diffs. */ +export type DependencyGraph = Map>; diff --git a/code/core/src/core-server/change-detection/dependency-graph/walkFromStory.ts b/code/core/src/core-server/change-detection/dependency-graph/walkFromStory.ts new file mode 100644 index 000000000000..678971d1a132 --- /dev/null +++ b/code/core/src/core-server/change-detection/dependency-graph/walkFromStory.ts @@ -0,0 +1,54 @@ +import type { ParserRegistry } from '../parser-registry/index.ts'; +import type { ParseResolveCache } from './ParseResolveCache.ts'; +import type { ReverseIndexImpl } from './ReverseIndex.ts'; + +export interface WalkFromStoryArgs { + storyRoot: string; + registry: ParserRegistry; + cache: ParseResolveCache; + reverseIndex: ReverseIndexImpl; + /** Called once per visited file with its resolved outgoing dependencies. */ + recordEdges: (file: string, deps: Set) => void; +} + +/** + * BFS forward-walk from `storyRoot`, parsing + resolving each file once via the shared + * cache and recording (dep, story, depth) tuples on `reverseIndex`. Stops at non-walkable + * files (no parser registered). + */ +export async function walkFromStory({ + storyRoot, + registry, + cache, + reverseIndex, + recordEdges, +}: WalkFromStoryArgs): Promise { + reverseIndex.record(storyRoot, storyRoot, 0); + + const visited = new Map(); + visited.set(storyRoot, 0); + const queue: Array<{ file: string; depth: number }> = [{ file: storyRoot, depth: 0 }]; + let head = 0; + + while (head < queue.length) { + const { file, depth } = queue[head++]; + + if (registry.parserFor(file) === undefined) { + continue; + } + + const resolvedDeps = await cache.resolveOnce(file); + recordEdges(file, resolvedDeps); + + const nextDepth = depth + 1; + for (const normalised of resolvedDeps) { + const previousDepth = visited.get(normalised); + if (previousDepth !== undefined && previousDepth <= nextDepth) { + continue; + } + visited.set(normalised, nextDepth); + reverseIndex.record(normalised, storyRoot, nextDepth); + queue.push({ file: normalised, depth: nextDepth }); + } + } +} diff --git a/code/core/src/core-server/change-detection/index.ts b/code/core/src/core-server/change-detection/index.ts index b0b545e148ba..1bd5583d15aa 100644 --- a/code/core/src/core-server/change-detection/index.ts +++ b/code/core/src/core-server/change-detection/index.ts @@ -6,3 +6,15 @@ export { type ChangeDetectionReadiness, } from './readiness.ts'; export { ChangeDetectionService } from './ChangeDetectionService.ts'; +export type { + ChangeDetectionAdapter, + FileChangeEvent, + ModuleResolveConfig, +} from './adapters/index.ts'; +export { ParserRegistry, builtinImportParsers } from './parser-registry/index.ts'; +export type { + ImportEdge, + ImportParser, + ImportParserContext, + ParseFileArgs, +} from './parser-registry/index.ts'; diff --git a/code/core/src/core-server/change-detection/parser-registry/ParserRegistry.test.ts b/code/core/src/core-server/change-detection/parser-registry/ParserRegistry.test.ts new file mode 100644 index 000000000000..1075bbb5a6fa --- /dev/null +++ b/code/core/src/core-server/change-detection/parser-registry/ParserRegistry.test.ts @@ -0,0 +1,134 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { logger } from 'storybook/internal/node-logger'; + +import { ParserRegistry } from './ParserRegistry.ts'; +import { builtinImportParsers } from './builtins.ts'; +import type { ImportParser } from './types.ts'; + +vi.mock('storybook/internal/node-logger', { spy: true }); + +describe('ParserRegistry', () => { + beforeEach(() => { + vi.mocked(logger.debug).mockImplementation(() => undefined); + }); + + it('registers default parsers and looks them up by extension', () => { + const registry = new ParserRegistry({ + defaultParsers: builtinImportParsers, + pluginParsers: [], + }); + + expect(registry.parserFor('/tmp/a.ts')).toBeDefined(); + expect(registry.parserFor('/tmp/a.tsx')).toBeDefined(); + expect(registry.parserFor('/tmp/a.mdx')).toBeDefined(); + }); + + it('lets a plugin parser override a default parser and logs a debug line', () => { + const pluginParse = vi.fn(async () => []); + const plugin: ImportParser = { + extensions: ['.ts'], + parse: pluginParse, + }; + + const registry = new ParserRegistry({ + defaultParsers: builtinImportParsers, + pluginParsers: [plugin], + }); + + expect(registry.parserFor('/tmp/a.ts')).toBe(pluginParse); + expect(vi.mocked(logger.debug)).toHaveBeenCalledWith( + expect.stringContaining('.ts parser overridden') + ); + }); + + it('returns undefined from parserFor for an unknown extension', () => { + const registry = new ParserRegistry({ + defaultParsers: builtinImportParsers, + pluginParsers: [], + }); + + expect(registry.parserFor('/tmp/a.unknown')).toBeUndefined(); + }); + + it('returns null from parse for an unknown extension (not empty array, not throw)', async () => { + const registry = new ParserRegistry({ + defaultParsers: builtinImportParsers, + pluginParsers: [], + }); + + await expect(registry.parse('/tmp/a.unknown', 'anything')).resolves.toBeNull(); + }); + + it('matches extensions case-insensitively (.TSX matches a .tsx plugin)', () => { + const plugin: ImportParser = { + extensions: ['.tsx'], + parse: vi.fn(async () => []), + }; + const registry = new ParserRegistry({ + defaultParsers: [], + pluginParsers: [plugin], + }); + + expect(registry.parserFor('/tmp/COMP.TSX')).toBe(plugin.parse); + }); + + it('lowercases the extensions a plugin registers so subsequent lookups match', () => { + const plugin: ImportParser = { + extensions: ['.FOO'], + parse: vi.fn(async () => []), + }; + const registry = new ParserRegistry({ + defaultParsers: [], + pluginParsers: [plugin], + }); + + expect(registry.parserFor('/tmp/a.foo')).toBe(plugin.parse); + }); + + it('exposes parseScriptWithOxc to plugins via the context, returning import edges', async () => { + let observedEdges: unknown; + const sfcPlugin: ImportParser = { + extensions: ['.sfc'], + async parse(_args, ctx) { + observedEdges = await ctx.parseScriptWithOxc( + `import x from 'y'; export { a } from 'b';`, + '/tmp/virtual.ts' + ); + return observedEdges as { specifier: string; kind: 'static' | 'dynamic' | 'require' }[]; + }, + }; + const registry = new ParserRegistry({ + defaultParsers: [], + pluginParsers: [sfcPlugin], + }); + + const edges = await registry.parse('/tmp/component.sfc', 'ignored-by-plugin'); + + expect(edges).toEqual([ + { specifier: 'y', kind: 'static' }, + { specifier: 'b', kind: 'static' }, + ]); + expect(observedEdges).toEqual(edges); + }); + + it('dispatches a known extension to the registered parser and returns its edges', async () => { + const pluginParse = vi.fn(async () => [{ specifier: 'foo', kind: 'static' as const }]); + const plugin: ImportParser = { + extensions: ['.foo'], + parse: pluginParse, + }; + const registry = new ParserRegistry({ + defaultParsers: [], + pluginParsers: [plugin], + }); + + const edges = await registry.parse('/tmp/a.foo', 'some source'); + + expect(edges).toEqual([{ specifier: 'foo', kind: 'static' }]); + expect(pluginParse).toHaveBeenCalledWith( + { filePath: '/tmp/a.foo', source: 'some source' }, + expect.objectContaining({ parseScriptWithOxc: expect.any(Function) }) + ); + }); +}); diff --git a/code/core/src/core-server/change-detection/parser-registry/ParserRegistry.ts b/code/core/src/core-server/change-detection/parser-registry/ParserRegistry.ts new file mode 100644 index 000000000000..79c30f9eb3a2 --- /dev/null +++ b/code/core/src/core-server/change-detection/parser-registry/ParserRegistry.ts @@ -0,0 +1,63 @@ +import { extname } from 'pathe'; + +import { logger } from 'storybook/internal/node-logger'; +import { parseWithOxc } from 'storybook/internal/oxc-parser'; + +import type { ImportEdge, ImportParser, ImportParserContext } from './types.ts'; + +/** + * Dispatches a file to the correct {@link ImportParser} based on its extension. The + * registry is constructed from a set of default parsers plus any plugin-supplied + * parsers (e.g. contributions to the `experimental_importParsers` preset key). + * + * Registration is last-wins on collision (plugin extensions override built-in + * extensions). Lookup is case-insensitive and uses `path.extname` — compound + * extensions like `.svelte.ts` match only the last segment (`.ts`). + */ +export class ParserRegistry { + private byExtension = new Map(); + private context: ImportParserContext; + + constructor(opts: { + defaultParsers: readonly ImportParser[]; + pluginParsers: readonly ImportParser[]; + }) { + this.context = { parseScriptWithOxc: this.parseScriptWithOxc.bind(this) }; + for (const p of opts.defaultParsers) { + this.register(p); + } + for (const p of opts.pluginParsers) { + this.register(p); + } + } + + private register(plugin: ImportParser): void { + for (const ext of plugin.extensions) { + const lower = ext.toLowerCase(); + if (this.byExtension.has(lower)) { + logger.debug(`ParserRegistry: ${lower} parser overridden`); + } + this.byExtension.set(lower, plugin.parse); + } + } + + parserFor(filePath: string): ImportParser['parse'] | undefined { + return this.byExtension.get(extname(filePath).toLowerCase()); + } + + /** + * Returns `null` when no parser claims the extension — callers interpret this as + * "opaque leaf, do not walk into". + */ + async parse(filePath: string, source: string): Promise { + const fn = this.parserFor(filePath); + if (!fn) { + return null; + } + return fn({ filePath, source }, this.context); + } + + private async parseScriptWithOxc(source: string, virtualFilePath: string): Promise { + return parseWithOxc(virtualFilePath, source); + } +} diff --git a/code/core/src/core-server/change-detection/parser-registry/builtins.test.ts b/code/core/src/core-server/change-detection/parser-registry/builtins.test.ts new file mode 100644 index 000000000000..2e009329c50f --- /dev/null +++ b/code/core/src/core-server/change-detection/parser-registry/builtins.test.ts @@ -0,0 +1,250 @@ +// Integration tests for the built-in ImportParser plugins. We exercise the real oxc-parser +// binary here (no spy) — the parser is fast enough for unit tests and stubbing it would +// test the stub, not the extractor's mapping from oxc nodes to ImportEdge. +import { describe, expect, it, vi } from 'vitest'; + +import { ChangeDetectionFailureError } from '../errors.ts'; +import { mdxImportParser, oxcImportParser } from './builtins.ts'; +import type { ImportParserContext } from './types.ts'; + +vi.mock('storybook/internal/node-logger', { spy: true }); + +const noopContext: ImportParserContext = { + parseScriptWithOxc: async () => [], +}; + +describe('oxcImportParser', () => { + it('claims the core JS/TS extensions', () => { + expect(oxcImportParser.extensions).toEqual(['.ts', '.tsx', '.js', '.jsx', '.mjs', '.cjs']); + }); + + it('extracts a regular static `import x from "y"`', async () => { + const edges = await oxcImportParser.parse( + { filePath: '/tmp/a.ts', source: `import x from 'y';` }, + noopContext + ); + + expect(edges).toEqual([{ specifier: 'y', kind: 'static' }]); + }); + + it('skips a type-only `import type x from "y"`', async () => { + const edges = await oxcImportParser.parse( + { filePath: '/tmp/a.ts', source: `import type x from 'y';` }, + noopContext + ); + + expect(edges).toEqual([]); + }); + + it('keeps a re-export `export { x } from "y"`', async () => { + const edges = await oxcImportParser.parse( + { filePath: '/tmp/a.ts', source: `export { x } from 'y';` }, + noopContext + ); + + expect(edges).toEqual([{ specifier: 'y', kind: 'static' }]); + }); + + it('skips a type-only `export type { x } from "y"`', async () => { + const edges = await oxcImportParser.parse( + { filePath: '/tmp/a.ts', source: `export type { x } from 'y';` }, + noopContext + ); + + expect(edges).toEqual([]); + }); + + it('keeps a dynamic `import("y")` with a string literal', async () => { + const edges = await oxcImportParser.parse( + { filePath: '/tmp/a.ts', source: `const m = import('y');` }, + noopContext + ); + + expect(edges).toEqual([{ specifier: 'y', kind: 'dynamic' }]); + }); + + it('skips a dynamic import with a non-literal specifier', async () => { + const edges = await oxcImportParser.parse( + { + filePath: '/tmp/a.ts', + source: `async function f(name: string) { await import(name); }`, + }, + noopContext + ); + + expect(edges).toEqual([]); + }); + + it('skips a template-literal dynamic import with interpolation', async () => { + const edges = await oxcImportParser.parse( + { + filePath: '/tmp/a.ts', + source: 'async function f(x: string) { await import(`./${x}`); }', + }, + noopContext + ); + + expect(edges).toEqual([]); + }); + + it('keeps a template-literal dynamic import with no interpolation', async () => { + const edges = await oxcImportParser.parse( + { filePath: '/tmp/a.ts', source: 'async function f() { await import(`./y`); }' }, + noopContext + ); + + expect(edges).toEqual([{ specifier: './y', kind: 'dynamic' }]); + }); + + it('extracts a `require("y")` call with a string literal', async () => { + const edges = await oxcImportParser.parse( + { filePath: '/tmp/a.js', source: `const m = require('y');` }, + noopContext + ); + + const requires = edges.filter((edge) => edge.kind === 'require'); + expect(requires).toEqual([{ specifier: 'y', kind: 'require' }]); + }); + + it('skips `require(someVar)` with a non-literal argument', async () => { + const edges = await oxcImportParser.parse( + { filePath: '/tmp/a.js', source: `function f(name) { return require(name); }` }, + noopContext + ); + + expect(edges.filter((edge) => edge.kind === 'require')).toEqual([]); + }); + + it('parses a .tsx file with JSX and an import together', async () => { + const edges = await oxcImportParser.parse( + { + filePath: '/tmp/a.tsx', + source: `import React from 'react';\nexport const X = () =>
;`, + }, + noopContext + ); + + expect(edges).toEqual([{ specifier: 'react', kind: 'static' }]); + }); + + it('parses `.mjs` and `.cjs` sources without error', async () => { + const mjs = await oxcImportParser.parse( + { filePath: '/tmp/a.mjs', source: `import x from 'y';` }, + noopContext + ); + const cjs = await oxcImportParser.parse( + { filePath: '/tmp/a.cjs', source: `const m = require('y');` }, + noopContext + ); + + expect(mjs).toEqual([{ specifier: 'y', kind: 'static' }]); + expect(cjs.filter((e) => e.kind === 'require')).toEqual([{ specifier: 'y', kind: 'require' }]); + }); + + it('wraps an oxc-parser throw in a ChangeDetectionFailureError', async () => { + // The dissolved oxc wrapper throws ChangeDetectionFailureError on any parser-level + // failure (null `module`, oxc throw). We cannot reliably produce an oxc throw with + // a test fixture — oxc is permissive. We document the contract: if the parser + // refused the source, the wrapping is a ChangeDetectionFailureError. + let threw: unknown; + try { + // Pass binary-garbage-ish source; if oxc refuses, we expect the error-wrapping path. + await oxcImportParser.parse( + { filePath: '/tmp/a.ts', source: ' invalid source `' }, + noopContext + ); + } catch (error) { + threw = error; + } + if (threw !== undefined) { + expect(threw).toBeInstanceOf(ChangeDetectionFailureError); + } + }); +}); + +describe('mdxImportParser', () => { + it('claims the `.mdx` extension', () => { + expect(mdxImportParser.extensions).toEqual(['.mdx']); + }); + + it('extracts top-of-file import lines from an MDX source', async () => { + const source = [ + `import { Meta } from '@storybook/blocks';`, + `import * as ButtonStories from './Button.stories';`, + ``, + ``, + ].join('\n'); + + const edges = await mdxImportParser.parse({ filePath: '/tmp/intro.mdx', source }, noopContext); + + expect(edges).toEqual([ + { specifier: '@storybook/blocks', kind: 'static' }, + { specifier: './Button.stories', kind: 'static' }, + ]); + }); + + it('returns an empty array when an MDX file has no imports', async () => { + const edges = await mdxImportParser.parse( + { filePath: '/tmp/doc.mdx', source: `# Title\n\nSome prose.` }, + noopContext + ); + + expect(edges).toEqual([]); + }); + + it('deduplicates identical import specifiers', async () => { + const source = [ + `import { Meta } from '@storybook/blocks';`, + `import { Canvas } from '@storybook/blocks';`, + ].join('\n'); + + const edges = await mdxImportParser.parse({ filePath: '/tmp/intro.mdx', source }, noopContext); + + expect(edges).toEqual([{ specifier: '@storybook/blocks', kind: 'static' }]); + }); + + it('extracts "export ... from" declarations from an MDX source', async () => { + const source = [`export { Meta } from '@storybook/blocks';`, `export * from './others';`].join( + '\n' + ); + + const edges = await mdxImportParser.parse({ filePath: '/tmp/intro.mdx', source }, noopContext); + + expect(edges).toEqual([ + { specifier: '@storybook/blocks', kind: 'static' }, + { specifier: './others', kind: 'static' }, + ]); + }); + + it('does NOT extract imports inside fenced code blocks (```js ... ```)', async () => { + const source = [ + `import { Meta } from '@storybook/blocks';`, + ``, + `\`\`\`js`, + `import fake from 'this-is-an-example';`, + `import another from './example-dep';`, + `\`\`\``, + ``, + ``, + ].join('\n'); + + const edges = await mdxImportParser.parse({ filePath: '/tmp/doc.mdx', source }, noopContext); + + expect(edges).toEqual([{ specifier: '@storybook/blocks', kind: 'static' }]); + expect(edges.map((e) => e.specifier)).not.toContain('this-is-an-example'); + expect(edges.map((e) => e.specifier)).not.toContain('./example-dep'); + }); + + it('does NOT extract imports inside backtick inline code spans', async () => { + const source = [ + `import { Canvas } from '@storybook/blocks';`, + ``, + `Use \`import foo from 'fake-pkg'\` in your code.`, + ].join('\n'); + + const edges = await mdxImportParser.parse({ filePath: '/tmp/doc.mdx', source }, noopContext); + + expect(edges).toEqual([{ specifier: '@storybook/blocks', kind: 'static' }]); + expect(edges.map((e) => e.specifier)).not.toContain('fake-pkg'); + }); +}); diff --git a/code/core/src/core-server/change-detection/parser-registry/builtins.ts b/code/core/src/core-server/change-detection/parser-registry/builtins.ts new file mode 100644 index 000000000000..108aedc76473 --- /dev/null +++ b/code/core/src/core-server/change-detection/parser-registry/builtins.ts @@ -0,0 +1,28 @@ +import { parseWithOxc } from 'storybook/internal/oxc-parser'; + +import { mdxParse } from './mdx-parse.ts'; +import type { ImportParser } from './types.ts'; + +/** Default parser for JavaScript/TypeScript source. Uses `oxc-parser` under the hood. */ +export const oxcImportParser: ImportParser = { + extensions: ['.ts', '.tsx', '.js', '.jsx', '.mjs', '.cjs'], + async parse({ filePath, source }) { + return parseWithOxc(filePath, source); + }, +}; + +/** Default parser for MDX files. Uses a regex fallback (oxc-parser cannot parse MDX). */ +export const mdxImportParser: ImportParser = { + extensions: ['.mdx'], + async parse({ source }) { + return mdxParse(source); + }, +}; + +/** + * Built-in parsers shipped with core change-detection. These cover the extensions the + * previous {@link ImportExtractor} handled directly; additional extensions (e.g. `.vue`, + * `.svelte`) are contributed by framework/renderer plugins via the + * `experimental_importParsers` preset key. + */ +export const builtinImportParsers: readonly ImportParser[] = [oxcImportParser, mdxImportParser]; diff --git a/code/core/src/core-server/change-detection/parser-registry/index.ts b/code/core/src/core-server/change-detection/parser-registry/index.ts new file mode 100644 index 000000000000..fe906b1f67a4 --- /dev/null +++ b/code/core/src/core-server/change-detection/parser-registry/index.ts @@ -0,0 +1,3 @@ +export type { ImportEdge, ImportParser, ImportParserContext, ParseFileArgs } from './types.ts'; +export { ParserRegistry } from './ParserRegistry.ts'; +export { builtinImportParsers, mdxImportParser, oxcImportParser } from './builtins.ts'; diff --git a/code/core/src/core-server/change-detection/parser-registry/mdx-parse.ts b/code/core/src/core-server/change-detection/parser-registry/mdx-parse.ts new file mode 100644 index 000000000000..bc07a8f4599b --- /dev/null +++ b/code/core/src/core-server/change-detection/parser-registry/mdx-parse.ts @@ -0,0 +1,43 @@ +import type { ImportEdge } from './types.ts'; + +/** + * Matches `import` or `export ... from` declarations in an MDX source. + * oxc-parser does not understand MDX, so we fall back to a regex. This only captures + * literal-string specifiers — enough for change detection to resolve them with oxc-resolver. + */ +const MDX_IMPORT_REGEX = + /(?:import\s+(?:[\s\S]*?\s+from\s+)?|export\s+[\s\S]*?\s+from\s+)['"]([^'"]+)['"]/g; + +/** + * Strips fenced code blocks (```lang\n...\n```) and inline code (`...`) from MDX source, + * replacing each region with spaces of equal length. Preserves line numbers so any + * future debug output remains accurate. We strip before running the import regex to avoid + * treating example imports inside code blocks as real dependency edges. + */ +function stripCodeRegions(source: string): string { + // Replace fenced code blocks first (they may contain backtick runs inside). + // The `s` flag makes `.` match newlines so multi-line blocks are covered. + let stripped = source.replace(/```[\s\S]*?```/gs, (match) => ' '.repeat(match.length)); + // Replace inline code spans (single backtick pairs). + stripped = stripped.replace(/`[^`]*`/g, (match) => ' '.repeat(match.length)); + return stripped; +} + +/** Extracts literal-string import edges from an `.mdx` file via regex fallback. */ +export function mdxParse(source: string): ImportEdge[] { + const stripped = stripCodeRegions(source); + const edges: ImportEdge[] = []; + const seen = new Set(); + MDX_IMPORT_REGEX.lastIndex = 0; + let match: RegExpExecArray | null = MDX_IMPORT_REGEX.exec(stripped); + while (match !== null) { + const specifier = match[1]; + const key = `static:${specifier}`; + if (!seen.has(key)) { + seen.add(key); + edges.push({ specifier, kind: 'static' }); + } + match = MDX_IMPORT_REGEX.exec(stripped); + } + return edges; +} diff --git a/code/core/src/core-server/change-detection/parser-registry/types.ts b/code/core/src/core-server/change-detection/parser-registry/types.ts new file mode 100644 index 000000000000..5e9eb43f33f1 --- /dev/null +++ b/code/core/src/core-server/change-detection/parser-registry/types.ts @@ -0,0 +1,38 @@ +// `ImportEdge` lives in the oxc-parser sub-package because it is the oxc-parser's +// output type, not a registry-specific concept. +import type { ImportEdge } from 'storybook/internal/oxc-parser'; + +export type { ImportEdge }; + +/** Arguments handed to an {@link ImportParser} when the registry dispatches a file to it. */ +export interface ParseFileArgs { + filePath: string; + source: string; +} + +/** + * Services passed to every parser. SFC parsers (vue, svelte) extract a `', + '', + '', + '', + '', + ].join('\n') + ); + + // New stories roll up to "Change status: New" on the parent component node + await expect(page.locator('[aria-label="Change status: New"]').first()).toBeVisible({ + timeout: CHANGE_DETECTION_TIMEOUT, + }); + } finally { + fs.rmSync(newStoryPath, { force: true }); + } + }); + + test('shows "new" status for an untracked story file (TypeScript)', async ({ page }) => { + test.skip( + templateName === 'svelte-vite/default-ts', + 'This test only works for TypeScript-based templates because it relies on creating a new .stories.ts file' + ); + const newStoryPath = path.join(sandboxDir, 'src/stories/ChangeDetectionNew.stories.ts'); + + console.log({ newStoryPath }); + + try { + fs.writeFileSync( + newStoryPath, + [ + "const meta = { title: 'Example/ChangeDetectionNew' }", + 'export default meta;', + '', + 'export const Default = {};', + ].join('\n') + ); + + // New stories roll up to "Change status: New" on the parent component node + await expect(page.locator('[aria-label="Change status: New"]').first()).toBeVisible({ + timeout: CHANGE_DETECTION_TIMEOUT, + }); + } finally { + fs.rmSync(newStoryPath, { force: true }); + } + }); + + test('shows "modified" status for a changed story file', async ({ page }) => { + test.skip(!headerStoriesPath, 'Header.stories file not found in STORYBOOK_SANDBOX_DIR'); + + const storyPath = headerStoriesPath as string; + const original = fs.readFileSync(storyPath, 'utf-8'); + + console.log({ storyPath }); + + try { + fs.writeFileSync(storyPath, `${original}\n// change-detection-e2e-modified`); + + await expect( + page.locator('[data-item-id="example-header"] [aria-label="Change status: Modified"]') + ).toBeVisible({ timeout: CHANGE_DETECTION_TIMEOUT }); + } finally { + fs.writeFileSync(storyPath, original); + } + }); + + test('shows "related" status for stories whose dependency changed', async ({ page }) => { + test.skip(!buttonComponentPath, 'Button component not found in STORYBOOK_SANDBOX_DIR'); + + const componentPath = buttonComponentPath as string; + const original = fs.readFileSync(componentPath, 'utf-8'); + // eslint-disable-next-line playwright/no-conditional-in-test + const comment = componentPath.endsWith('.svelte') + ? '\n' + : '\n// change-detection-e2e-affected'; + + try { + fs.writeFileSync(componentPath, `${original}${comment}`); + + // Header.stories is depth-2 from Button.tsx (via Header.tsx); depth-1 Button.stories gets "Modified" + await expect( + page.locator('[data-item-id="example-header"] [aria-label="Change status: Affected"]') + ).toBeVisible({ timeout: CHANGE_DETECTION_TIMEOUT }); + } finally { + fs.writeFileSync(componentPath, original); + } + }); +}); diff --git a/code/lib/cli-storybook/src/sandbox-templates.ts b/code/lib/cli-storybook/src/sandbox-templates.ts index c169252cf1da..dae8d4e5e079 100644 --- a/code/lib/cli-storybook/src/sandbox-templates.ts +++ b/code/lib/cli-storybook/src/sandbox-templates.ts @@ -246,6 +246,7 @@ export const baseTemplates = { experimentalRSC: true, developmentModeForBuild: true, experimentalTestSyntax: true, + changeDetection: true, }, }, extraDependencies: ['server-only', 'prop-types'], @@ -343,6 +344,7 @@ export const baseTemplates = { experimentalRSC: true, developmentModeForBuild: true, experimentalTestSyntax: true, + changeDetection: true, }, }, extraDependencies: ['server-only', 'vite', 'prop-types'], @@ -385,6 +387,7 @@ export const baseTemplates = { features: { developmentModeForBuild: true, experimentalTestSyntax: true, + changeDetection: true, }, }, }, diff --git a/code/lib/create-storybook/src/generators/REACT_NATIVE/metroConfig.test.ts b/code/lib/create-storybook/src/generators/REACT_NATIVE/metroConfig.test.ts index ee91b9c0dac0..5d904e53c84b 100644 --- a/code/lib/create-storybook/src/generators/REACT_NATIVE/metroConfig.test.ts +++ b/code/lib/create-storybook/src/generators/REACT_NATIVE/metroConfig.test.ts @@ -2,6 +2,7 @@ import { mkdtemp, readFile, rm, writeFile } from 'node:fs/promises'; import os from 'node:os'; import path from 'node:path'; +import { format } from 'oxfmt'; import { logger, prompt } from 'storybook/internal/node-logger'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; @@ -19,6 +20,15 @@ import { vi.mock('storybook/internal/node-logger', { spy: true }); +const getFormattedDiff = async (before: string, after: string, filePath: string) => { + const [beforeResult, afterResult] = await Promise.all([ + format(path.basename(filePath), before, { singleQuote: true }), + format(path.basename(filePath), after, { singleQuote: true }), + ]); + + return getDiff(beforeResult.code, afterResult.code); +}; + describe('metroConfig codemod', () => { let tempDir: string; let originalCwd: string; @@ -56,10 +66,8 @@ describe('metroConfig codemod', () => { expect(result.status).toBe('updated'); expect(after).not.toBe(before); - expect(getDiff(before, after)).toMatchInlineSnapshot(` - "+ const { - + withStorybook, - + } = require('@storybook/react-native/withStorybook'); + expect(await getFormattedDiff(before, after, filePath)).toMatchInlineSnapshot(` + "+ const { withStorybook } = require('@storybook/react-native/withStorybook'); + + const defaultConfig = {}; @@ -84,32 +92,29 @@ describe('metroConfig codemod', () => { expect(result.status).toBe('updated'); expect(after).not.toBe(before); - expect(getDiff(before, after)).toMatchInlineSnapshot(` + expect(await getFormattedDiff(before, after, filePath)).toMatchInlineSnapshot(` "- module.exports = withExpo(defaultConfig); - - + const { - + withStorybook, - + } = require('@storybook/react-native/withStorybook'); + + const { withStorybook } = require('@storybook/react-native/withStorybook'); + + module.exports = withStorybook(withExpo(defaultConfig)); + " `); }); - it('wraps export default async function in ts config with ESM import', () => { + it('wraps export default async function in ts config with ESM import', async () => { + const filePath = path.join(tempDir, 'metro.config.ts'); const before = ` export default async function makeMetroConfig() { return { resolver: {} }; } `; - const transformed = transformMetroConfigSource(before, path.join(tempDir, 'metro.config.ts')); + const transformed = transformMetroConfigSource(before, filePath); expect(transformed.action).toBe('updated'); if (transformed.action === 'updated') { - expect(getDiff(before, transformed.code)).toMatchInlineSnapshot(` - " - - - export default async function makeMetroConfig() { + expect(await getFormattedDiff(before, transformed.code, filePath)).toMatchInlineSnapshot(` + "- export default async function makeMetroConfig() { - + import { withStorybook } from '@storybook/react-native/withStorybook'; + export default withStorybook(async function makeMetroConfig() { @@ -195,7 +200,8 @@ module.exports = config; } }); - it('does not treat import/export text inside template literals as ESM for .ts import injection', () => { + it('does not treat import/export text inside template literals as ESM for .ts import injection', async () => { + const filePath = path.join(tempDir, 'metro.config.ts'); const before = ` const hint = \` import { something } from 'somewhere' @@ -203,16 +209,12 @@ export default {} \`; module.exports = {}; `; - const transformed = transformMetroConfigSource(before, path.join(tempDir, 'metro.config.ts')); + const transformed = transformMetroConfigSource(before, filePath); expect(transformed.action).toBe('updated'); if (transformed.action === 'updated') { - expect(getDiff(before, transformed.code)).toMatchInlineSnapshot(` - " - - + const { - + withStorybook, - + } = require('@storybook/react-native/withStorybook'); + expect(await getFormattedDiff(before, transformed.code, filePath)).toMatchInlineSnapshot(` + "+ const { withStorybook } = require('@storybook/react-native/withStorybook'); + + const hint = \` @@ -228,7 +230,8 @@ module.exports = {}; } }); - it('preserves TypeScript return type and type parameters on default exported functions', () => { + it('preserves TypeScript return type and type parameters on default exported functions', async () => { + const filePath = path.join(tempDir, 'metro.config.ts'); const before = ` import type { MetroConfig } from 'metro-config'; @@ -236,13 +239,12 @@ export default async function makeMetroConfig(): Promise(): Promise { diff --git a/code/package.json b/code/package.json index b2931a6253d6..2b6aeaff053b 100644 --- a/code/package.json +++ b/code/package.json @@ -196,5 +196,6 @@ "Dependency Upgrades" ] ] - } + }, + "deferredNextVersion": "10.4.0-alpha.17" } diff --git a/code/renderers/svelte/src/parsers/index.ts b/code/renderers/svelte/src/parsers/index.ts new file mode 100644 index 000000000000..e6d9f24d9330 --- /dev/null +++ b/code/renderers/svelte/src/parsers/index.ts @@ -0,0 +1 @@ +export { svelteImportParser } from './svelteImportParser.ts'; diff --git a/code/renderers/svelte/src/parsers/svelteImportParser.test.ts b/code/renderers/svelte/src/parsers/svelteImportParser.test.ts new file mode 100644 index 000000000000..da913c7d257d --- /dev/null +++ b/code/renderers/svelte/src/parsers/svelteImportParser.test.ts @@ -0,0 +1,248 @@ +import { describe, expect, it, vi } from 'vitest'; + +import type { ImportEdge, ImportParserContext } from 'storybook/internal/core-server'; +import { ChangeDetectionFailureError } from 'storybook/internal/core-server'; + +import { svelteImportParser } from './svelteImportParser.ts'; + +function makeContext(behavior: (source: string, virtualFilePath: string) => ImportEdge[]): { + ctx: ImportParserContext; + calls: { source: string; virtualFilePath: string }[]; +} { + const calls: { source: string; virtualFilePath: string }[] = []; + const ctx: ImportParserContext = { + parseScriptWithOxc: vi.fn(async (source: string, virtualFilePath: string) => { + calls.push({ source, virtualFilePath }); + return behavior(source, virtualFilePath); + }), + }; + return { ctx, calls }; +} + +describe('svelteImportParser', () => { + it('claims the `.svelte` extension', () => { + expect(svelteImportParser.extensions).toEqual(['.svelte']); + }); + + it('does NOT claim `.svelte.ts` / `.svelte.js` rune files (ParserRegistry routes those to oxc by last extension segment)', () => { + // `path.extname('/tmp/Rune.svelte.ts')` returns `.ts`, so rune files fall through + // to the built-in oxc parser. This assertion documents that contract: we must not + // register `.svelte.ts` / `.svelte.js` here. + expect(svelteImportParser.extensions).not.toContain('.svelte.ts'); + expect(svelteImportParser.extensions).not.toContain('.svelte.js'); + }); + + it('extracts imports from a regular `, + ``, + `
hello
`, + ].join('\n'); + + const { ctx, calls } = makeContext((src) => { + if (src.includes(`from './Button.svelte'`)) { + return [ + { specifier: './Button.svelte', kind: 'static' }, + { specifier: 'svelte', kind: 'static' }, + ]; + } + return []; + }); + + const edges = await svelteImportParser.parse({ filePath: '/tmp/Foo.svelte', source }, ctx); + + expect(calls).toHaveLength(1); + expect(calls[0]?.virtualFilePath).toBe('/tmp/Foo.svelte.script.js'); + expect(calls[0]?.source).toContain(`import Button from './Button.svelte';`); + expect(calls[0]?.source).toContain(`import { onMount } from 'svelte';`); + expect(calls[0]?.source).not.toContain('
'); + expect(edges).toEqual([ + { specifier: './Button.svelte', kind: 'static' }, + { specifier: 'svelte', kind: 'static' }, + ]); + }); + + it('extracts imports from a `, + ``, + `

body

`, + ].join('\n'); + + const { ctx, calls } = makeContext((src) => { + if (src.includes(`from './store.ts'`)) { + return [{ specifier: './store.ts', kind: 'static' }]; + } + return []; + }); + + const edges = await svelteImportParser.parse({ filePath: '/tmp/Bar.svelte', source }, ctx); + + expect(calls).toHaveLength(1); + expect(calls[0]?.virtualFilePath).toBe('/tmp/Bar.svelte.script.js'); + expect(calls[0]?.source).toContain(`import { store } from './store.ts';`); + expect(edges).toEqual([{ specifier: './store.ts', kind: 'static' }]); + }); + + it('extracts imports from BOTH instance and module scripts and dedupes', async () => { + const source = [ + ``, + ``, + ``, + ``, + `
`, + ].join('\n'); + + const { ctx, calls } = makeContext((src) => { + if (src.includes(`from './shared.ts'`)) { + return [ + { specifier: './shared.ts', kind: 'static' }, + { specifier: './Button.svelte', kind: 'static' }, + ]; + } + return [ + { specifier: 'svelte', kind: 'static' }, + { specifier: './Button.svelte', kind: 'static' }, + ]; + }); + + const edges = await svelteImportParser.parse({ filePath: '/tmp/Baz.svelte', source }, ctx); + + expect(calls).toHaveLength(2); + expect(edges).toEqual([ + { specifier: './shared.ts', kind: 'static' }, + { specifier: './Button.svelte', kind: 'static' }, + { specifier: 'svelte', kind: 'static' }, + ]); + }); + + it('returns [] for a Svelte file with only markup and styles', async () => { + const source = [ + `
`, + `

No scripts here.

`, + `
`, + ``, + ``, + ].join('\n'); + + const { ctx, calls } = makeContext(() => []); + + const edges = await svelteImportParser.parse({ filePath: '/tmp/Empty.svelte', source }, ctx); + + expect(calls).toHaveLength(0); + expect(edges).toEqual([]); + }); + + it('forwards type-only import filtering to parseScriptWithOxc (no special-casing)', async () => { + // svelteImportParser does not know about type-only imports — that's oxc's job. We + // assert here that the parser hands the script verbatim to parseScriptWithOxc and + // returns exactly what oxc reports, so type-only filtering is preserved end to end. + const source = [ + ``, + ].join('\n'); + + const { ctx, calls } = makeContext(() => [{ specifier: 'svelte/store', kind: 'static' }]); + + const edges = await svelteImportParser.parse({ filePath: '/tmp/Typed.svelte', source }, ctx); + + expect(calls).toHaveLength(1); + expect(calls[0]?.source).toContain(`import type { Writable } from 'svelte/store';`); + expect(calls[0]?.source).toContain(`import { writable } from 'svelte/store';`); + expect(edges).toEqual([{ specifier: 'svelte/store', kind: 'static' }]); + }); + + it('parses a .stories.svelte (Svelte CSF) file the same way', async () => { + const source = [ + ``, + ``, + ``, + ].join('\n'); + + const { ctx } = makeContext(() => [ + { specifier: '@storybook/addon-svelte-csf', kind: 'static' }, + { specifier: './Button.svelte', kind: 'static' }, + ]); + + const edges = await svelteImportParser.parse( + { filePath: '/tmp/Button.stories.svelte', source }, + ctx + ); + + expect(edges).toEqual([ + { specifier: '@storybook/addon-svelte-csf', kind: 'static' }, + { specifier: './Button.svelte', kind: 'static' }, + ]); + }); + + it('surfaces a malformed .svelte source as ChangeDetectionFailureError', async () => { + // Unclosed tag + bad expression — should cause svelte/compiler to throw. + const source = ``].join('\n'); + + const { ctx, calls } = makeContext(() => [{ specifier: './x.js', kind: 'static' }]); + + await svelteImportParser.parse({ filePath: '/tmp/NoLang.svelte', source }, ctx); + + expect(calls[0]?.virtualFilePath).toBe('/tmp/NoLang.svelte.script.js'); + }); + + it('uses .script.ts virtual extension when lang="ts"', async () => { + const source = [``].join('\n'); + + const { ctx, calls } = makeContext(() => [{ specifier: './x.ts', kind: 'static' }]); + + await svelteImportParser.parse({ filePath: '/tmp/TypeScript.svelte', source }, ctx); + + expect(calls[0]?.virtualFilePath).toBe('/tmp/TypeScript.svelte.script.ts'); + }); + + it('uses .script.tsx virtual extension when lang="tsx"', async () => { + const source = [``].join('\n'); + + const { ctx, calls } = makeContext(() => [{ specifier: './x.tsx', kind: 'static' }]); + + await svelteImportParser.parse({ filePath: '/tmp/TSX.svelte', source }, ctx); + + expect(calls[0]?.virtualFilePath).toBe('/tmp/TSX.svelte.script.tsx'); + }); + + it('uses .script.jsx virtual extension when lang="jsx"', async () => { + const source = [``].join('\n'); + + const { ctx, calls } = makeContext(() => [{ specifier: './x.jsx', kind: 'static' }]); + + await svelteImportParser.parse({ filePath: '/tmp/JSX.svelte', source }, ctx); + + expect(calls[0]?.virtualFilePath).toBe('/tmp/JSX.svelte.script.jsx'); + }); +}); diff --git a/code/renderers/svelte/src/parsers/svelteImportParser.ts b/code/renderers/svelte/src/parsers/svelteImportParser.ts new file mode 100644 index 000000000000..165a914ce157 --- /dev/null +++ b/code/renderers/svelte/src/parsers/svelteImportParser.ts @@ -0,0 +1,178 @@ +import { + ChangeDetectionFailureError, + type ImportEdge, + type ImportParser, +} from 'storybook/internal/core-server'; + +type SvelteCompiler = typeof import('svelte/compiler'); + +let svelteCompilerPromise: Promise | undefined; + +async function getSvelteCompiler(): Promise { + if (!svelteCompilerPromise) { + svelteCompilerPromise = import('svelte/compiler').catch((error: unknown) => { + svelteCompilerPromise = undefined; + throw new ChangeDetectionFailureError( + `Failed to load 'svelte/compiler'; is 'svelte' installed? Original error: ${ + error instanceof Error ? error.message : String(error) + }`, + error instanceof Error ? { cause: error } : undefined + ); + }); + } + return svelteCompilerPromise; +} + +interface MaybeProgramRange { + start?: number; + end?: number; +} + +/** Loose shape of a single attribute node from the Svelte AST (`Attribute[]` on `Script`). */ +interface MaybeAstAttribute { + name?: unknown; + /** `true` for boolean attrs; array of text/expression nodes for string attrs. */ + value?: unknown; +} + +interface MaybeScript { + content?: MaybeProgramRange | null; + /** Svelte AST `Script.attributes` — an array of `Attribute` nodes. */ + attributes?: MaybeAstAttribute[] | null; +} + +interface MaybeSvelteRoot { + instance?: MaybeScript | null; + module?: MaybeScript | null; +} + +/** + * Reads the `lang` attribute from a Svelte AST `Script.attributes` array. + * Each attribute is `{ name, value }` where value is `true` (boolean attr) or an array + * of text/expression nodes — `[{ data: "ts" }]` for `lang="ts"`. + */ +function readLangFromAttributes( + attributes: MaybeAstAttribute[] | null | undefined +): string | undefined { + if (!Array.isArray(attributes)) { + return undefined; + } + for (const attr of attributes) { + if (attr.name !== 'lang') { + continue; + } + // Boolean attribute (`lang` without value) — no practical meaning, skip. + if (attr.value === true) { + return undefined; + } + // String attribute: value is an array of text/expression nodes. + if (Array.isArray(attr.value) && attr.value.length > 0) { + const first = attr.value[0] as { data?: unknown } | undefined; + if (typeof first?.data === 'string') { + return first.data; + } + } + return undefined; + } + return undefined; +} + +function extractScriptSource( + source: string, + script: MaybeScript | null | undefined +): { scriptSource: string; lang: string | undefined } | undefined { + const range = script?.content; + if (!range || typeof range.start !== 'number' || typeof range.end !== 'number') { + return undefined; + } + if (range.start < 0 || range.end > source.length || range.end <= range.start) { + return undefined; + } + const lang = readLangFromAttributes(script?.attributes); + return { scriptSource: source.slice(range.start, range.end), lang }; +} + +/** + * Maps the `lang` attribute of a ``, + ].join('\n'); + + const { ctx, calls } = makeContext(() => [{ specifier: 'vue', kind: 'static' }]); + + const edges = await vueImportParser.parse({ filePath: '/tmp/Button.vue', source }, ctx); + + expect(calls).toHaveLength(1); + expect(calls[0]?.virtualFilePath).toBe('/tmp/Button.vue.script.ts'); + expect(calls[0]?.source).toContain(`import { defineComponent } from 'vue';`); + expect(calls[0]?.source).toContain(`import type { PropType } from 'vue';`); + expect(calls[0]?.source).not.toContain('