Skip to content

fix: resolve site asset paths without breaking on Windows - #21

Open
SpookyMoo wants to merge 1 commit into
mainfrom
fix/windows-asset-path-test
Open

SpookyMoo wants to merge 1 commit into
mainfrom
fix/windows-asset-path-test

Conversation

@SpookyMoo

Copy link
Copy Markdown
Owner

Test-only change. No site content touched.

The bug

all local page assets exist built its path with:

join(new URL(root).pathname, target)

On Windows new URL(root).pathname is /C:/Users/..., with a leading slash, so every path it built was invalid and the test failed on any Windows checkout even though the files were all present. CI runs on Linux, so the suite looked green and the failure only ever showed up locally.

The fix

Resolve against the base URL and convert properly:

existsSync(fileURLToPath(new URL(`.${target}`, root)))

The . prefix matters. new URL("/assets/theme.css", root) would resolve against the filesystem root instead of the site directory.

join was used only on that line, so the node:path import is replaced with node:url.

Verification

  • npm test on Windows: 10 of 10 pass. It was 9 of 10 before this change, on this same commit of main.
  • Confirmed the check is not now vacuously passing. Resolution lands on C:\Users\Owner\kernel-kittens-site\site\assets\theme.css; a real asset returns true and a fabricated one returns false, so a genuinely missing asset still fails the build.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VcZ4MQ1967Hho27oj8ZkWj

The asset existence check built a path from new URL(root).pathname,
which on Windows yields /C:/Users/... with a leading slash. Every
checkout on Windows failed "all local page assets exist" even when the
files were present. CI runs on Linux, so the suite looked green.

Resolve the URL properly and convert with fileURLToPath instead. The
check still fails for a genuinely missing asset, verified both ways.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants