diff --git a/code/lib/core-server/src/utils/stories-json.test.ts b/code/lib/core-server/src/utils/stories-json.test.ts index 5ba134673c83..f770986cd2f6 100644 --- a/code/lib/core-server/src/utils/stories-json.test.ts +++ b/code/lib/core-server/src/utils/stories-json.test.ts @@ -356,12 +356,17 @@ describe('useStoriesJson', () => { expect(Watchpack).toHaveBeenCalledTimes(1); const watcher = Watchpack.mock.instances[0]; - expect(watcher.watch).toHaveBeenCalledWith({ directories: ['./src'] }); + expect(watcher.watch).toHaveBeenCalledWith( + expect.objectContaining({ + directories: expect.any(Array), + files: expect.any(Array), + }) + ); expect(watcher.on).toHaveBeenCalledTimes(2); const onChange = watcher.on.mock.calls[0][1]; - await onChange('src/nested/Button.stories.ts'); + await onChange(`${workingDir}/src/nested/Button.stories.ts`); expect(mockServerChannel.emit).toHaveBeenCalledTimes(1); expect(mockServerChannel.emit).toHaveBeenCalledWith(STORY_INDEX_INVALIDATED); }); @@ -389,12 +394,17 @@ describe('useStoriesJson', () => { expect(Watchpack).toHaveBeenCalledTimes(1); const watcher = Watchpack.mock.instances[0]; - expect(watcher.watch).toHaveBeenCalledWith({ directories: ['./src'] }); + expect(watcher.watch).toHaveBeenCalledWith( + expect.objectContaining({ + directories: expect.any(Array), + files: expect.any(Array), + }) + ); expect(watcher.on).toHaveBeenCalledTimes(2); const onChange = watcher.on.mock.calls[0][1]; - await onChange('src/nested/Button.stories.ts'); + await onChange(`${workingDir}/src/nested/Button.stories.ts`); expect(mockServerChannel.emit).toHaveBeenCalledTimes(1); expect(mockServerChannel.emit).toHaveBeenCalledWith(STORY_INDEX_INVALIDATED); }); @@ -423,16 +433,21 @@ describe('useStoriesJson', () => { expect(Watchpack).toHaveBeenCalledTimes(1); const watcher = Watchpack.mock.instances[0]; - expect(watcher.watch).toHaveBeenCalledWith({ directories: ['./src'] }); + expect(watcher.watch).toHaveBeenCalledWith( + expect.objectContaining({ + directories: expect.any(Array), + files: expect.any(Array), + }) + ); expect(watcher.on).toHaveBeenCalledTimes(2); const onChange = watcher.on.mock.calls[0][1]; - await onChange('src/nested/Button.stories.ts'); - await onChange('src/nested/Button.stories.ts'); - await onChange('src/nested/Button.stories.ts'); - await onChange('src/nested/Button.stories.ts'); - await onChange('src/nested/Button.stories.ts'); + await onChange(`${workingDir}/src/nested/Button.stories.ts`); + await onChange(`${workingDir}/src/nested/Button.stories.ts`); + await onChange(`${workingDir}/src/nested/Button.stories.ts`); + await onChange(`${workingDir}/src/nested/Button.stories.ts`); + await onChange(`${workingDir}/src/nested/Button.stories.ts`); expect(mockServerChannel.emit).toHaveBeenCalledTimes(1); expect(mockServerChannel.emit).toHaveBeenCalledWith(STORY_INDEX_INVALIDATED); diff --git a/code/lib/core-server/src/utils/watch-story-specifiers.test.ts b/code/lib/core-server/src/utils/watch-story-specifiers.test.ts index 4026de15fa11..4a150cc0e7e2 100644 --- a/code/lib/core-server/src/utils/watch-story-specifiers.test.ts +++ b/code/lib/core-server/src/utils/watch-story-specifiers.test.ts @@ -13,6 +13,7 @@ describe('watchStorySpecifiers', () => { configDir: path.join(workingDir, '.storybook'), workingDir, }; + const abspath = (filename: string) => path.join(workingDir, filename); let close: () => void; afterEach(() => close?.()); @@ -25,11 +26,18 @@ describe('watchStorySpecifiers', () => { expect(Watchpack).toHaveBeenCalledTimes(1); const watcher = Watchpack.mock.instances[0]; - expect(watcher.watch).toHaveBeenCalledWith({ directories: ['./src'] }); + expect(watcher.watch).toHaveBeenCalledWith( + expect.objectContaining({ + directories: expect.any(Array), + files: expect.any(Array), + }) + ); expect(watcher.on).toHaveBeenCalledTimes(2); - const onChange = watcher.on.mock.calls[0][1]; - const onRemove = watcher.on.mock.calls[1][1]; + const baseOnChange = watcher.on.mock.calls[0][1]; + const baseOnRemove = watcher.on.mock.calls[1][1]; + const onChange = (filename: string, ...args: any[]) => baseOnChange(abspath(filename), ...args); + const onRemove = (filename: string, ...args: any[]) => baseOnRemove(abspath(filename), ...args); // File changed, matching onInvalidate.mockClear(); @@ -72,10 +80,16 @@ describe('watchStorySpecifiers', () => { expect(Watchpack).toHaveBeenCalledTimes(1); const watcher = Watchpack.mock.instances[0]; - expect(watcher.watch).toHaveBeenCalledWith({ directories: ['./src'] }); + expect(watcher.watch).toHaveBeenCalledWith( + expect.objectContaining({ + directories: expect.any(Array), + files: expect.any(Array), + }) + ); expect(watcher.on).toHaveBeenCalledTimes(2); - const onChange = watcher.on.mock.calls[0][1]; + const baseOnChange = watcher.on.mock.calls[0][1]; + const onChange = (filename: string, ...args: any[]) => baseOnChange(abspath(filename), ...args); onInvalidate.mockClear(); await onChange('src/nested', 1234); @@ -90,11 +104,18 @@ describe('watchStorySpecifiers', () => { expect(Watchpack).toHaveBeenCalledTimes(1); const watcher = Watchpack.mock.instances[0]; - expect(watcher.watch).toHaveBeenCalledWith({ directories: ['./src/nested'] }); + expect(watcher.watch).toHaveBeenCalledWith( + expect.objectContaining({ + directories: expect.any(Array), + files: expect.any(Array), + }) + ); expect(watcher.on).toHaveBeenCalledTimes(2); - const onChange = watcher.on.mock.calls[0][1]; - const onRemove = watcher.on.mock.calls[1][1]; + const baseOnChange = watcher.on.mock.calls[0][1]; + const baseOnRemove = watcher.on.mock.calls[1][1]; + const onChange = (filename: string, ...args: any[]) => baseOnChange(abspath(filename), ...args); + const onRemove = (filename: string, ...args: any[]) => baseOnRemove(abspath(filename), ...args); // File changed, matching onInvalidate.mockClear(); @@ -131,10 +152,16 @@ describe('watchStorySpecifiers', () => { expect(Watchpack).toHaveBeenCalledTimes(1); const watcher = Watchpack.mock.instances[0]; - expect(watcher.watch).toHaveBeenCalledWith({ directories: ['./src', './src/nested'] }); + expect(watcher.watch).toHaveBeenCalledWith( + expect.objectContaining({ + directories: expect.any(Array), + files: expect.any(Array), + }) + ); expect(watcher.on).toHaveBeenCalledTimes(2); - const onChange = watcher.on.mock.calls[0][1]; + const baseOnChange = watcher.on.mock.calls[0][1]; + const onChange = (filename: string, ...args: any[]) => baseOnChange(abspath(filename), ...args); onInvalidate.mockClear(); await onChange('src/nested/Button.stories.ts', 1234); diff --git a/code/lib/core-server/src/utils/watch-story-specifiers.ts b/code/lib/core-server/src/utils/watch-story-specifiers.ts index 70ec0d4eb7fb..414fd4c87617 100644 --- a/code/lib/core-server/src/utils/watch-story-specifiers.ts +++ b/code/lib/core-server/src/utils/watch-story-specifiers.ts @@ -2,7 +2,6 @@ import Watchpack from 'watchpack'; import slash from 'slash'; import fs from 'fs'; import path from 'path'; -import uniq from 'lodash/uniq.js'; import type { NormalizedStoriesSpecifier, Path } from '@storybook/types'; import { commonGlobOptions } from '@storybook/core-common'; @@ -15,11 +14,27 @@ const isDirectory = (directory: Path) => { } }; -// Watchpack (and path.relative) passes paths either with no leading './' - e.g. `src/Foo.stories.js`, -// or with a leading `../` (etc), e.g. `../src/Foo.stories.js`. -// We want to deal in importPaths relative to the working dir, so we normalize -function toImportPath(relativePath: Path) { - return relativePath.startsWith('.') ? relativePath : `./${relativePath}`; +// Takes an array of absolute paths to directories and synchronously returns +// absolute paths to all existing files and directories nested within those +// directories (including the passed parent directories). +function getNestedFilesAndDirectories(directories: Path[]) { + const traversedDirectories = new Set(); + const files = new Set(); + const traverse = (directory: Path) => { + if (traversedDirectories.has(directory)) { + return; + } + fs.readdirSync(directory, { withFileTypes: true }).forEach((ent: fs.Dirent) => { + if (ent.isDirectory()) { + traverse(path.join(directory, ent.name)); + } else if (ent.isFile()) { + files.add(path.join(directory, ent.name)); + } + }); + traversedDirectories.add(directory); + }; + directories.filter(isDirectory).forEach(traverse); + return { files: Array.from(files), directories: Array.from(traversedDirectories) }; } export function watchStorySpecifiers( @@ -27,6 +42,12 @@ export function watchStorySpecifiers( options: { workingDir: Path }, onInvalidate: (specifier: NormalizedStoriesSpecifier, path: Path, removed: boolean) => void ) { + // Watch all nested files and directories up front to avoid this issue: + // https://github.com/webpack/watchpack/issues/222 + const { files, directories } = getNestedFilesAndDirectories( + specifiers.map((ns) => path.resolve(options.workingDir, ns.directory)) + ); + // See https://www.npmjs.com/package/watchpack for full options. // If you want less traffic, consider using aggregation with some interval const wp = new Watchpack({ @@ -34,15 +55,17 @@ export function watchStorySpecifiers( followSymlinks: false, ignored: ['**/.git', '**/node_modules'], }); - wp.watch({ - directories: uniq(specifiers.map((ns) => ns.directory)), - }); + wp.watch({ files, directories }); + + const toImportPath = (absolutePath: Path) => { + const relativePath = path.relative(options.workingDir, absolutePath); + return slash(relativePath.startsWith('.') ? relativePath : `./${relativePath}`); + }; - async function onChangeOrRemove(watchpackPath: Path, removed: boolean) { - // Watchpack passes paths either with no leading './' - e.g. `src/Foo.stories.js`, - // or with a leading `../` (etc), e.g. `../src/Foo.stories.js`. - // We want to deal in importPaths relative to the working dir, or absolute paths. - const importPath = slash(watchpackPath.startsWith('.') ? watchpackPath : `./${watchpackPath}`); + async function onChangeOrRemove(absolutePath: Path, removed: boolean) { + // Watchpack should return absolute paths, given we passed in absolute paths + // to watch. Convert to an import path so we can run against the specifiers. + const importPath = toImportPath(absolutePath); const matchingSpecifier = specifiers.find((ns) => ns.importPathMatcher.exec(importPath)); if (matchingSpecifier) { @@ -55,7 +78,6 @@ export function watchStorySpecifiers( // However, when a directory is added, it does not fire events for any files *within* the directory, // so we need to scan within that directory for new files. It is tricky to use a glob for this, // so we'll do something a bit more "dumb" for now - const absolutePath = path.join(options.workingDir, importPath); if (!removed && isDirectory(absolutePath)) { await Promise.all( specifiers @@ -66,11 +88,10 @@ export function watchStorySpecifiers( // If `./path/to/dir` was added, check all files matching `./path/to/dir/**/*.stories.*` // (where the last bit depends on `files`). const dirGlob = path.join( - options.workingDir, - importPath, + absolutePath, '**', // files can be e.g. '**/foo/*/*.js' so we just want the last bit, - // because the directoru could already be within the files part (e.g. './x/foo/bar') + // because the directory could already be within the files part (e.g. './x/foo/bar') path.basename(specifier.files) ); @@ -78,13 +99,10 @@ export function watchStorySpecifiers( const { globby } = await import('globby'); // glob only supports forward slashes - const files = await globby(slash(dirGlob), commonGlobOptions(dirGlob)); + const addedFiles = await globby(slash(dirGlob), commonGlobOptions(dirGlob)); - files.forEach((filePath) => { - const fileImportPath = toImportPath( - // use posix path separators even on windows - path.relative(options.workingDir, filePath).replace(/\\/g, '/') - ); + addedFiles.forEach((filePath: Path) => { + const fileImportPath = toImportPath(filePath); if (specifier.importPathMatcher.exec(fileImportPath)) { onInvalidate(specifier, fileImportPath, removed);