Core: Fix startup hang caused by watchStorySpecifiers

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);

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,34 +14,58 @@ 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<Path>();
+  const files = new Set<Path>();
+  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(
   specifiers: NormalizedStoriesSpecifier[],
   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({
     // poll: true, // Slow!!! Enable only in special cases
     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,25 +88,21 @@ 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)
             );
 
             // Dynamically import globby because it is a pure ESM module
             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);