From e744798b1e56ef874c8fb39f4ed6987285c3039b Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Tue, 3 Dec 2024 10:33:47 +1100 Subject: [PATCH] Ensure we store and pass `storybookBaseDir` as a unix path everywhere --- node-src/lib/getDependentStoryFiles.ts | 10 ++----- .../lib/getStorybookBaseDirectory.test.ts | 30 ++++++++++++++++++- node-src/lib/getStorybookBaseDirectory.ts | 3 +- node-src/lib/posix.ts | 7 +++++ 4 files changed, 40 insertions(+), 10 deletions(-) create mode 100644 node-src/lib/posix.ts diff --git a/node-src/lib/getDependentStoryFiles.ts b/node-src/lib/getDependentStoryFiles.ts index 851356324..5c85aa0ff 100644 --- a/node-src/lib/getDependentStoryFiles.ts +++ b/node-src/lib/getDependentStoryFiles.ts @@ -4,6 +4,7 @@ import { Context, Module, Reason, Stats } from '../types'; import noCSFGlobs from '../ui/messages/errors/noCSFGlobs'; import tracedAffectedFiles from '../ui/messages/info/tracedAffectedFiles'; import bailFile from '../ui/messages/warnings/bailFile'; +import { posix } from './posix'; import { isPackageManifestFile, matchesFile } from './utils'; type FilePath = string; @@ -27,11 +28,6 @@ const isUserModule = (module_: Module | Reason) => (module_ as Module).id !== null && !INTERNALS.some((re) => re.test((module_ as Module).name || (module_ as Reason).moduleName)); -// Replaces Windows-style backslash path separators with POSIX-style forward slashes, because the -// Webpack stats use forward slashes in the `name` and `moduleName` fields. Note `changedFiles` -// already contains forward slashes, because that's what git yields even on Windows. -const posix = (localPath: string) => localPath.split(path.sep).filter(Boolean).join(path.posix.sep); - // For any path in node_modules, return the package name, including scope prefix if any. const getPackageName = (modulePath: string) => { const [, scopedName] = modulePath.match(/\/node_modules\/(@[\w-]+\/[\w-]+)\//) || []; @@ -90,7 +86,7 @@ export async function getDependentStoryFiles( } const { - baseDir: storybookBaseDirectory = '', + baseDir: baseDirectory = '', configDir: configDirectory = '.storybook', staticDir: staticDirectory = [], viewLayer, @@ -102,8 +98,6 @@ export async function getDependentStoryFiles( untraced = [], } = ctx.options; - const baseDirectory = posix(storybookBaseDirectory); - // Convert a "webpack path" (relative to storybookBaseDir) to a "git path" (relative to repository root) // e.g. `./src/file.js` => `path/to/storybook/src/file.js` const normalize = (posixPath: FilePath): NormalizedName => { diff --git a/node-src/lib/getStorybookBaseDirectory.test.ts b/node-src/lib/getStorybookBaseDirectory.test.ts index 3581f0068..e727ab98e 100644 --- a/node-src/lib/getStorybookBaseDirectory.test.ts +++ b/node-src/lib/getStorybookBaseDirectory.test.ts @@ -1,11 +1,20 @@ +import path from 'node:path'; import process from 'node:process'; -import { expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { Context } from '../types'; import { getStorybookBaseDirectory } from './getStorybookBaseDirectory'; const mockedCwd = vi.spyOn(process, 'cwd'); +const mockedRelative = vi.spyOn(path, 'relative'); +const mockedJoin = vi.spyOn(path, 'join'); + +// The definition of posix depends on `path.sep` being correct for the system +// (ie.equal to `\\` for windows), however we can't really mock that as it's a constant +vi.mock('./posix', () => ({ + posix: (localPath: string) => localPath.split('\\').filter(Boolean).join('/'), +})); it('defaults to the configured value', () => { expect(getStorybookBaseDirectory({ options: { storybookBaseDir: 'foobar' } } as Context)).toBe( @@ -40,3 +49,22 @@ it('falls back the empty string if there is no git root', () => { expect(getStorybookBaseDirectory({} as Context)).toBe('.'); }); + +describe('with windows paths', () => { + beforeEach(() => { + mockedRelative.mockImplementation(path.win32.relative); + mockedJoin.mockImplementation(path.win32.join); + }); + + afterEach(() => { + mockedRelative.mockRestore(); + mockedJoin.mockRestore(); + }); + + it('uses posix paths even if we are windows', () => { + const rootPath = String.raw`C:\path\to\project`; + mockedCwd.mockReturnValue(String.raw`${rootPath}\storybook\subdir`); + + expect(getStorybookBaseDirectory({ git: { rootPath } } as Context)).toBe('storybook/subdir'); + }); +}); diff --git a/node-src/lib/getStorybookBaseDirectory.ts b/node-src/lib/getStorybookBaseDirectory.ts index a00e1efe7..9e9ed6d48 100644 --- a/node-src/lib/getStorybookBaseDirectory.ts +++ b/node-src/lib/getStorybookBaseDirectory.ts @@ -1,6 +1,7 @@ import path from 'path'; import { Context } from '../types'; +import { posix } from './posix'; /** * Get the storybook base directory, relative to the git root. @@ -24,5 +25,5 @@ export function getStorybookBaseDirectory(ctx: Context) { // NOTE: // - path.relative does not have a leading '.', unless it starts with '../' // - path.join('.', '') === '.' and path.join('.', '../x') = '../x' - return path.join('.', path.relative(rootPath, '')); + return posix(path.join('.', path.relative(rootPath, ''))); } diff --git a/node-src/lib/posix.ts b/node-src/lib/posix.ts new file mode 100644 index 000000000..48ba80cfa --- /dev/null +++ b/node-src/lib/posix.ts @@ -0,0 +1,7 @@ +import path from 'path'; + +// Replaces Windows-style backslash path separators with POSIX-style forward slashes, because the +// Webpack stats use forward slashes in the `name` and `moduleName` fields. Note `changedFiles` +// already contains forward slashes, because that's what git yields even on Windows. +export const posix = (localPath: string) => + localPath.split(path.sep).filter(Boolean).join(path.posix.sep);