Fix how paths are normalized

chromaui · Jan 28, 2022 · e770baf · e770baf
1 parent 4b6cf13
commit e770baf
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 29 deletions.
diff --git a/bin-src/lib/getDependentStoryFiles.js b/bin-src/lib/getDependentStoryFiles.js
@@ -1,6 +1,6 @@
 import path from 'path';
 
-import { getWorkingDir, matchesFile } from './utils';
+import { matchesFile } from './utils';
 import { getRepositoryRoot } from '../git/git';
 import bailFile from '../ui/messages/warnings/bailFile';
 import noCSFGlobs from '../ui/messages/errors/noCSFGlobs';
@@ -33,14 +33,14 @@ const posix = (localPath) => localPath.split(path.sep).filter(Boolean).join(path
 /**
  * Converts a module path found in the webpack stats to be relative to the (git) root path. Module
  * paths can be relative (`./module.js`) or absolute (`/path/to/project/module.js`). The webpack
- * stats may have been generated in a subdirectory, so we prepend the workingDir if necessary.
+ * stats may have been generated in a subdirectory, so we prepend the baseDir if necessary.
  * The result is a relative POSIX path compatible with `git diff --name-only`.
  */
-export function normalizePath(posixPath, rootPath, workingDir = '') {
+export function normalizePath(posixPath, rootPath, baseDir = '') {
   if (!posixPath) return posixPath;
   return path.posix.isAbsolute(posixPath)
     ? path.posix.relative(rootPath, posixPath)
-    : path.posix.join(workingDir, posixPath);
+    : path.posix.join(baseDir, posixPath);
 }
 
 /**
@@ -58,8 +58,10 @@ export async function getDependentStoryFiles(ctx, stats, statsPath, changedFiles
   } = ctx.options;
 
   const rootPath = await getRepositoryRoot(); // e.g. `/path/to/project` (always absolute posix)
-  const workingDir = getWorkingDir(rootPath, storybookBaseDir); // e.g. `packages/storybook` or empty string
-  const normalize = (posixPath) => normalizePath(posixPath, rootPath, workingDir); // e.g. `src/file.js` (no ./ prefix)
+  const baseDir = storybookBaseDir ? posix(storybookBaseDir) : path.posix.relative(rootPath, '');
+
+  // Convert a "webpack path" (relative to storybookBaseDir) to a "git path" (relative to repository root)
+  const normalize = (posixPath) => normalizePath(posixPath, rootPath, baseDir); // e.g. `src/file.js` (no ./ prefix)
 
   const storybookDir = normalize(posix(storybookConfigDir));
   const staticDirs = staticDir.map((dir) => normalize(posix(dir)));
@@ -145,7 +147,7 @@ export async function getDependentStoryFiles(ctx, stats, statsPath, changedFiles
 
   ctx.turboSnap = {
     rootPath,
-    workingDir,
+    baseDir,
     storybookDir,
     staticDirs,
     globs,

diff --git a/bin-src/lib/getDependentStoryFiles.test.js b/bin-src/lib/getDependentStoryFiles.test.js
@@ -2,19 +2,19 @@ import chalk from 'chalk';
 
 import { getDependentStoryFiles, normalizePath } from './getDependentStoryFiles';
 import { getRepositoryRoot } from '../git/git';
-import { getWorkingDir } from './utils';
 
 jest.mock('../git/git');
-jest.mock('./utils', () => {
-  const utils = jest.requireActual('./utils');
-  return { __esModule: true, ...utils, getWorkingDir: jest.fn() };
-});
 
 const CSF_GLOB = './src sync ^\\.\\/(?:(?!\\.)(?=.)[^/]*?\\.stories\\.js)$';
 const statsPath = 'preview-stats.json';
 
 const log = { info: jest.fn(), warn: jest.fn(), error: jest.fn(), debug: jest.fn() };
-const getContext = (ctx) => ({ log, options: {}, turboSnap: {}, ...ctx });
+const getContext = ({ configDir, staticDir, ...options } = {}) => ({
+  log,
+  options: { storybookBaseDir: '.', ...options },
+  turboSnap: {},
+  storybook: { configDir, staticDir },
+});
 
 afterEach(() => {
   log.info.mockReset();
@@ -24,7 +24,6 @@ afterEach(() => {
 });
 
 getRepositoryRoot.mockResolvedValue('/path/to/project');
-getWorkingDir.mockReturnValue('');
 
 describe('getDependentStoryFiles', () => {
   it('detects direct changes to CSF files', async () => {
@@ -139,7 +138,6 @@ describe('getDependentStoryFiles', () => {
   });
 
   it('supports webpack projectRoot in git subdirectory', async () => {
-    getWorkingDir.mockReturnValueOnce('services/webapp');
     const changedFiles = ['services/webapp/src/foo.js'];
     const modules = [
       {
@@ -158,7 +156,7 @@ describe('getDependentStoryFiles', () => {
         reasons: [{ moduleName: './.storybook/generated-stories-entry.js' }],
       },
     ];
-    const ctx = getContext();
+    const ctx = getContext({ storybookBaseDir: 'services/webapp' });
     const res = await getDependentStoryFiles(ctx, { modules }, statsPath, changedFiles);
     expect(res).toEqual({
       './src/foo.stories.js': ['services/webapp/src/foo.stories.js'],
@@ -196,7 +194,6 @@ describe('getDependentStoryFiles', () => {
 
   it('supports absolute module paths with deviating working dir', async () => {
     getRepositoryRoot.mockResolvedValueOnce('/path/to/project');
-    getWorkingDir.mockReturnValueOnce('packages/storybook'); // note this is a different workspace
 
     const absoluteCsfGlob = `/path/to/project/packages/webapp/${CSF_GLOB.slice(2)}`;
     const changedFiles = ['packages/webapp/src/foo.js'];
@@ -218,7 +215,7 @@ describe('getDependentStoryFiles', () => {
         reasons: [{ moduleName: './.storybook/generated-stories-entry.js' }],
       },
     ];
-    const ctx = getContext();
+    const ctx = getContext({ storybookBaseDir: 'packages/storybook' });
     const res = await getDependentStoryFiles(ctx, { modules }, statsPath, changedFiles);
     expect(res).toEqual({
       '/path/to/project/packages/webapp/src/foo.stories.js': ['packages/webapp/src/foo.stories.js'],
@@ -234,7 +231,7 @@ describe('getDependentStoryFiles', () => {
         reasons: [{ moduleName: './path/to/storybook-config/file.js' }],
       },
     ];
-    const ctx = getContext({ storybook: { configDir: 'path/to/storybook-config' } });
+    const ctx = getContext({ configDir: 'path/to/storybook-config' });
     await expect(() =>
       getDependentStoryFiles(ctx, { modules }, statsPath, changedFiles)
     ).rejects.toEqual(new Error('Did not find any CSF globs in preview-stats.json'));
@@ -279,7 +276,7 @@ describe('getDependentStoryFiles', () => {
         reasons: [{ moduleName: './path/to/storybook-config/generated-stories-entry.js' }],
       },
     ];
-    const ctx = getContext({ storybook: { configDir: 'path/to/storybook-config' } });
+    const ctx = getContext({ configDir: 'path/to/storybook-config' });
     const res = await getDependentStoryFiles(ctx, { modules }, statsPath, changedFiles);
     expect(res).toEqual(null);
     expect(ctx.turboSnap.bailReason).toEqual({
@@ -306,7 +303,7 @@ describe('getDependentStoryFiles', () => {
         reasons: [{ moduleName: './path/to/storybook-config/generated-stories-entry.js' }],
       },
     ];
-    const ctx = getContext({ storybook: { configDir: 'path/to/storybook-config' } });
+    const ctx = getContext({ configDir: 'path/to/storybook-config' });
     const res = await getDependentStoryFiles(ctx, { modules }, statsPath, changedFiles);
     expect(res).toEqual(null);
     expect(ctx.turboSnap.bailReason).toEqual({
@@ -334,7 +331,7 @@ describe('getDependentStoryFiles', () => {
         reasons: [{ moduleName: './path/to/storybook-config/generated-stories-entry.js' }],
       },
     ];
-    const ctx = getContext({ storybook: { configDir: 'path/to/storybook-config' } });
+    const ctx = getContext({ configDir: 'path/to/storybook-config' });
     const res = await getDependentStoryFiles(ctx, { modules }, statsPath, changedFiles);
     expect(res).toEqual(null);
     expect(ctx.turboSnap.bailReason).toEqual({
@@ -361,7 +358,7 @@ describe('getDependentStoryFiles', () => {
         reasons: [{ moduleName: './.storybook/generated-stories-entry.js' }],
       },
     ];
-    const ctx = getContext({ storybook: { staticDir: ['path/to/statics'] } });
+    const ctx = getContext({ staticDir: ['path/to/statics'] });
     const res = await getDependentStoryFiles(ctx, { modules }, statsPath, changedFiles);
     expect(res).toEqual(null);
     expect(ctx.turboSnap.bailReason).toEqual({
@@ -397,8 +394,8 @@ describe('getDependentStoryFiles', () => {
       },
     ];
     const ctx = getContext({
-      storybook: { staticDir: ['path/to/statics'] },
-      options: { untraced: ['**/foo.js'] },
+      staticDir: ['path/to/statics'],
+      untraced: ['**/foo.js'],
     });
     const res = await getDependentStoryFiles(ctx, { modules }, statsPath, changedFiles);
     expect(res).toEqual({});

diff --git a/bin-src/lib/utils.js b/bin-src/lib/utils.js
@@ -1,4 +1,3 @@
-import path from 'path';
 import picomatch from 'picomatch';
 
 export const lcfirst = (str) => `${str.charAt(0).toLowerCase()}${str.substr(1)}`;
@@ -34,9 +33,6 @@ export const rewriteErrorMessage = (err, message) => {
   }
 };
 
-export const getWorkingDir = (basePath, targetPath = '.') =>
-  path.posix.relative(basePath, targetPath);
-
 const fileMatchers = {};
 export const matchesFile = (glob, filepath) => {
   if (!fileMatchers[glob]) fileMatchers[glob] = picomatch(glob, { dot: true });