From f5265ec7df2df4335a6322e5d59fb0467d96f808 Mon Sep 17 00:00:00 2001 From: Aryan Kansagara Date: Thu, 3 Sep 2026 14:03:26 -0400 Subject: [PATCH 1/2] fix(cli): decode percent-encoded monorepo path `monorepoPackagesPath` was read from `new URL(...).pathname`, which is percent-encoded. When the repository is checked out to a path containing a space, the encoded path does not exist on disk, so `isHydrogenMonorepo` is false and `getSkeletonSourceDir()` throws "Trying to use skeleton source dir outside of Hydrogen monorepo." Decode the pathname before use. `fileURLToPath` is still avoided here for the Windows backslash reasons noted in the existing comment. --- .changeset/olive-pugs-shave.md | 6 ++++++ packages/cli/src/lib/build.test.ts | 18 ++++++++++++++++++ packages/cli/src/lib/build.ts | 12 ++++++++++-- 3 files changed, 34 insertions(+), 2 deletions(-) create mode 100644 .changeset/olive-pugs-shave.md create mode 100644 packages/cli/src/lib/build.test.ts diff --git a/.changeset/olive-pugs-shave.md b/.changeset/olive-pugs-shave.md new file mode 100644 index 0000000000..e5291c265b --- /dev/null +++ b/.changeset/olive-pugs-shave.md @@ -0,0 +1,6 @@ +--- +'@shopify/cli-hydrogen': patch +'@shopify/create-hydrogen': patch +--- + +Fix Hydrogen monorepo detection when the repository path contains spaces or other characters that are percent-encoded in URLs. The path is now decoded before it is used to look for the skeleton template. diff --git a/packages/cli/src/lib/build.test.ts b/packages/cli/src/lib/build.test.ts new file mode 100644 index 0000000000..dafbb865b2 --- /dev/null +++ b/packages/cli/src/lib/build.test.ts @@ -0,0 +1,18 @@ +import {describe, it, expect} from 'vitest'; +import {decodedPathname} from './build.js'; + +describe('decodedPathname()', () => { + it('decodes percent-encoded characters', () => { + expect( + decodedPathname( + new URL('file:///Users/x/Open%20Source/hydrogen/packages/'), + ), + ).toBe('/Users/x/Open Source/hydrogen/packages/'); + }); + + it('leaves paths without encoded characters untouched', () => { + expect(decodedPathname(new URL('file:///Users/x/hydrogen/packages/'))).toBe( + '/Users/x/hydrogen/packages/', + ); + }); +}); diff --git a/packages/cli/src/lib/build.ts b/packages/cli/src/lib/build.ts index 6aeeb37c5d..e72c9b30ae 100644 --- a/packages/cli/src/lib/build.ts +++ b/packages/cli/src/lib/build.ts @@ -5,8 +5,16 @@ import {AbortError} from '@shopify/cli-kit/node/error'; import {dirname, joinPath} from '@shopify/cli-kit/node/path'; import {execAsync} from './process.js'; -// Avoid using fileURLToPath here to prevent backslashes nightmare on Windows -const monorepoPackagesPath = new URL('../../..', import.meta.url).pathname; +// Avoid using fileURLToPath here to prevent backslashes nightmare on Windows. +// URL pathnames are percent-encoded, so decode to support directories that +// contain spaces or other reserved characters. +export function decodedPathname(url: URL) { + return decodeURIComponent(url.pathname); +} + +const monorepoPackagesPath = decodedPathname( + new URL('../../..', import.meta.url), +); // Check if we're in the Hydrogen monorepo by checking the path structure // This was the original logic before PR #3074 that worked for over a year From c99e1fb604644593fa427af527c5080b1a18b28e Mon Sep 17 00:00:00 2001 From: Aryan Kansagara Date: Fri, 4 Sep 2026 12:00:46 -0400 Subject: [PATCH 2/2] test(cli): cover monorepo detection instead of the decode helper The previous test only called the decoding helper directly, so it stayed green if production went back to consuming raw `.pathname`. Take the module URL as an argument in `getMonorepoPackagesPath` and `detectHydrogenMonorepo` so detection can run against a real directory, and assert against a temporary checkout whose path contains a space. Verified the test fails both when the decode is removed from the helper and when detection bypasses the helper entirely. --- packages/cli/src/lib/build.test.ts | 60 +++++++++++++++++++++++------- packages/cli/src/lib/build.ts | 19 +++++----- 2 files changed, 57 insertions(+), 22 deletions(-) diff --git a/packages/cli/src/lib/build.test.ts b/packages/cli/src/lib/build.test.ts index dafbb865b2..911cdd87d1 100644 --- a/packages/cli/src/lib/build.test.ts +++ b/packages/cli/src/lib/build.test.ts @@ -1,18 +1,52 @@ import {describe, it, expect} from 'vitest'; -import {decodedPathname} from './build.js'; - -describe('decodedPathname()', () => { - it('decodes percent-encoded characters', () => { - expect( - decodedPathname( - new URL('file:///Users/x/Open%20Source/hydrogen/packages/'), - ), - ).toBe('/Users/x/Open Source/hydrogen/packages/'); +import {pathToFileURL} from 'node:url'; +import {inTemporaryDirectory, mkdir} from '@shopify/cli-kit/node/fs'; +import {joinPath} from '@shopify/cli-kit/node/path'; +import {detectHydrogenMonorepo, getMonorepoPackagesPath} from './build.js'; + +// Mirrors the location of this file inside the monorepo, since monorepo +// detection resolves `packages/` relative to the module that asks for it. +const MODULE_PATH_IN_REPO = ['packages', 'cli', 'src', 'lib', 'build.ts']; + +function moduleUrlFor(repoRoot: string) { + return pathToFileURL(joinPath(repoRoot, ...MODULE_PATH_IN_REPO)); +} + +async function createFakeMonorepo(repoRoot: string) { + await mkdir(joinPath(repoRoot, ...MODULE_PATH_IN_REPO.slice(0, -1))); + await mkdir(joinPath(repoRoot, 'templates', 'skeleton')); +} + +describe('monorepo detection', () => { + it('detects the monorepo when the path contains a space', async () => { + await inTemporaryDirectory(async (tmpDir) => { + // The space is what regressed: `URL.pathname` percent-encodes it, and + // the encoded path does not exist on disk. + const repoRoot = joinPath(tmpDir, 'Open Source', 'hydrogen'); + await createFakeMonorepo(repoRoot); + + expect(detectHydrogenMonorepo(moduleUrlFor(repoRoot))).toBe(true); + expect(getMonorepoPackagesPath(moduleUrlFor(repoRoot))).toContain( + 'Open Source', + ); + }); }); - it('leaves paths without encoded characters untouched', () => { - expect(decodedPathname(new URL('file:///Users/x/hydrogen/packages/'))).toBe( - '/Users/x/hydrogen/packages/', - ); + it('detects the monorepo when the path contains no space', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const repoRoot = joinPath(tmpDir, 'hydrogen'); + await createFakeMonorepo(repoRoot); + + expect(detectHydrogenMonorepo(moduleUrlFor(repoRoot))).toBe(true); + }); + }); + + it('does not detect a monorepo without templates/skeleton', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const repoRoot = joinPath(tmpDir, 'Open Source', 'not-hydrogen'); + await mkdir(joinPath(repoRoot, ...MODULE_PATH_IN_REPO.slice(0, -1))); + + expect(detectHydrogenMonorepo(moduleUrlFor(repoRoot))).toBe(false); + }); }); }); diff --git a/packages/cli/src/lib/build.ts b/packages/cli/src/lib/build.ts index e72c9b30ae..99b1505972 100644 --- a/packages/cli/src/lib/build.ts +++ b/packages/cli/src/lib/build.ts @@ -5,25 +5,22 @@ import {AbortError} from '@shopify/cli-kit/node/error'; import {dirname, joinPath} from '@shopify/cli-kit/node/path'; import {execAsync} from './process.js'; +// Resolve the `packages/` directory that contains this file. // Avoid using fileURLToPath here to prevent backslashes nightmare on Windows. // URL pathnames are percent-encoded, so decode to support directories that // contain spaces or other reserved characters. -export function decodedPathname(url: URL) { - return decodeURIComponent(url.pathname); +export function getMonorepoPackagesPath(moduleUrl: string | URL) { + return decodeURIComponent(new URL('../../..', moduleUrl).pathname); } -const monorepoPackagesPath = decodedPathname( - new URL('../../..', import.meta.url), -); - // Check if we're in the Hydrogen monorepo by checking the path structure // This was the original logic before PR #3074 that worked for over a year // Check if we're in the monorepo by looking for the templates/skeleton directory // This works both in development and when built as npm package -export const isHydrogenMonorepo = (() => { +export function detectHydrogenMonorepo(moduleUrl: string | URL) { try { const skeletonPath = joinPath( - dirname(monorepoPackagesPath), + dirname(getMonorepoPackagesPath(moduleUrl)), 'templates', 'skeleton', ); @@ -33,7 +30,11 @@ export const isHydrogenMonorepo = (() => { } catch { return false; } -})(); +} + +const monorepoPackagesPath = getMonorepoPackagesPath(import.meta.url); + +export const isHydrogenMonorepo = detectHydrogenMonorepo(import.meta.url); export const hydrogenPackagesPath = isHydrogenMonorepo ? monorepoPackagesPath : undefined;