Shopify / Shopify/hydrogen

CLI build and tests fail when the repo is cloned to a path containing spaces

Open
#3,988 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
TypeScript
Stars
2.1k
Forks
443
Avg merge
4d 19h
Merged PRs (30d)
24

Description

Which package or tool is having this issue?

CLI

What version of that package or tool are you using?

main @ b543c6d2 (@shopify/cli-hydrogen 13.0.4)

Steps to Reproduce

Clone the repo into a directory whose path contains a space, then build:

git clone https://github.com/Shopify/hydrogen.git "/tmp/Open Source/hydrogen"
cd "/tmp/Open Source/hydrogen"
pnpm install
pnpm --dir packages/cli build
Expected Behavior

The build succeeds, the same as it does from a path without spaces.

Actual Behavior

The build fails:

Error: Trying to use skeleton source dir outside of Hydrogen monorepo.
    at getSkeletonSourceDir (packages/cli/tsup.config.bundled_*.mjs:36:11)
    at Object.onSuccess (packages/cli/tsup.config.bundled_*.mjs:166:18)

pnpm --dir packages/cli test fails as well, with 11 failed test files and 39 failed tests on the same checkout.

Cause is in packages/cli/src/lib/build.ts:

const monorepoPackagesPath = new URL('../../..', import.meta.url).pathname;

URL.pathname is percent-encoded, so a checkout at /Users/me/Open Source/hydrogen yields /Users/me/Open%20Source/hydrogen/packages/. That path does not exist on disk, so the existsSync check for templates/skeleton fails, isHydrogenMonorepo is false, and getSkeletonSourceDir() throws.

The error message points at the monorepo layout rather than the directory name, so the actual cause is not obvious. The fix is to decode the pathname before use.

Confirmed on the same machine by toggling only that one expression:

packages/cli build packages/cli tests
Current main fails 11 files / 39 tests failed
With the pathname decoded succeeds 57 files / 434 tests passed

Note that packages/hydrogen/src/vite/plugin.ts uses the same new URL(...).pathname pattern, but is not affected, because it only checks endsWith('/packages/') and any encoded segment appears earlier in the path.

This only affects working in the monorepo. Installed npm packages are unaffected, since isHydrogenMonorepo is false there either way.

I have a fix ready and will open a PR shortly.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in packages/cli/src/lib/build.ts and inspect how the monorepo package path is derived from import.meta.url. Reproduce with a checkout under a path containing spaces, then run pnpm --dir packages/cli build and pnpm --dir packages/cli test. Done means the build succeeds and the CLI tests pass from the spaced path.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
cli, testing
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.