Skip to content

Commit

Permalink
Ensure we store and pass storybookBaseDir as a unix path everywhere
Browse files Browse the repository at this point in the history
  • Loading branch information
tmeasday committed Dec 2, 2024
1 parent 6daedce commit e744798
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 10 deletions.
10 changes: 2 additions & 8 deletions node-src/lib/getDependentStoryFiles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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-]+)\//) || [];
Expand Down Expand Up @@ -90,7 +86,7 @@ export async function getDependentStoryFiles(
}

const {
baseDir: storybookBaseDirectory = '',
baseDir: baseDirectory = '',
configDir: configDirectory = '.storybook',
staticDir: staticDirectory = [],
viewLayer,
Expand All @@ -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 => {
Expand Down
30 changes: 29 additions & 1 deletion node-src/lib/getStorybookBaseDirectory.test.ts
Original file line number Diff line number Diff line change
@@ -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(
Expand Down Expand Up @@ -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');
});
});
3 changes: 2 additions & 1 deletion node-src/lib/getStorybookBaseDirectory.ts
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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, '')));

Check warning on line 28 in node-src/lib/getStorybookBaseDirectory.ts

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

node-src/lib/getStorybookBaseDirectory.ts#L28

Detected possible user input going into a `path.join` or `path.resolve` function.
}
7 changes: 7 additions & 0 deletions node-src/lib/posix.ts
Original file line number Diff line number Diff line change
@@ -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);

0 comments on commit e744798

Please sign in to comment.