Sign up
View stargazersSign up

fix(web): the web build works from a checkout path with spaces#30666

closed2
main
GitHub fork: fix/web-build-path-with-space
opened7d agobyUUmais-Adeed
1 files changed
View on GitHubTwo-way sync enabledPR last imported 34m ago

Problem

vp build in apps/web fails when the checkout path contains a space. The @betteroffice/fonts-cjk alias in apps/web/vite.config.ts was resolved with new URL(...).pathname, which percent-encodes the space as %20, so the aliased file does not exist. Fixes #29587.

Change

Resolve the alias with NodeURL.fileURLToPath(new URL(...)), which decodes the path, as the issue suggests.

Scope and approval

One-line fix for the build bug reported in #29587.

Verification

Built apps/web from a fresh checkout at /home/…/t3 space repro on e68e19d605:

  • Before: vp build exits 1 with [UNLOADABLE_DEPENDENCY] Could not load ../../../t3%20space%20repro/apps/web/src/components/files/office/fonts-cjk.ts … No such file or directory.
  • After: vp build exits 0, ✓ built in 16.02s.
  • vp run check:changed: format, lint, knip, and typecheck pass.

No test added: this is build configuration, and the build in CI covers it. No UI change.

Done with Claude Opus 5.5 in Claude Code (T3 Code).

🤖 Generated with Claude Code

Checks

22 successful · 16 skipped
All checks passed
e553f9f

Added 1 commit

  • fix(web): the web build works from a checkout path with spaces51ca6c0
MmacroscopeappBot7d ago
Synced from GitHub

Approvability

Verdict: Approved at 51ca6c0

Macroscope's review found this PR approvable — The web build now converts the existing font alias URL into a native filesystem path, supporting checkout directories containing spaces. The alias target and font behavior remain unchanged, with no material operational exposure.

Notes:

  • All code in this push has already been reviewed. Approvability was decided on eligibility alone.

You can add or adjust custom eligibility rules. Learn more.

CcoderabbitaiBot7d ago
Synced from GitHub

Explain Change →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

External image Run configuration

External image Commits

Reviewing files that changed from the base of the PR and between 51ca6c0594a1e3b334e3aae7b2abf59b34136623 and e553f9f4dcc66a325a4db2795b4eed111f7946ce.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • Trigger review

No actionable comments were generated in the recent review.

Walkthrough

The Vite configuration imports node:url and converts the fonts-CJK alias URL to a filesystem path with fileURLToPath.

Changes: Web fonts-CJK alias

Layer / File(s)Summary
Convert the alias URL to a filesystem path
apps/web/vite.config.ts		The fonts-CJK alias now uses fileURLToPath instead of the URL’s .pathname.

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix · Severity of issue fixed: Low

Possibly related PRs

Suggested reviewers: maria-rcks

Merge Risk: External image Minimal · previous review 51ca6

The path-space build issue is addressed without introducing additional merge-blocking risk.

Pre-merge checks: Passed 4

Check nameStatusExplanation
Title checkPassedThe title clearly and concisely describes the primary change: fixing the web build when the checkout path contains spaces.
Description checkPassedThe description includes the required Problem, Change, Scope and approval, and Verification sections. It explains the failure, the fix, the linked issue, the focused scope, and the before-and-after bu…
Linked Issues checkPassed[#29587] The only coding requirement is that the web build succeeds from a checkout path that contains a space. The root cause is that the @betteroffice/fonts-cjk alias in apps/web/vite.config.ts …
Out of Scope Changes checkPassedThe whole-PR diff changes one file, apps/web/vite.config.ts. It adds the node:url import and changes the fonts-CJK alias expression. Both edits serve the objective of #29587. No unrelated file or …
Full details: Description check

Explanation

The description includes the required Problem, Change, Scope and approval, and Verification sections. It explains the failure, the fix, the linked issue, the focused scope, and the before-and-after build results.

Full details: Linked Issues check

Explanation

[#29587] The only coding requirement is that the web build succeeds from a checkout path that contains a space. The root cause is that the @betteroffice/fonts-cjk alias in apps/web/vite.config.ts used new URL(...).pathname, which keeps %20 in the path. The diff at merge base e68e19d605 to head 51ca6c0594 replaces this with NodeURL.fileURLToPath(new URL("./src/components/files/office/fonts-cjk.ts", import.meta.url)), with import * as NodeURL from "node:url" added. This is the fix the issue proposes. fileURLToPath decodes the percent-encoded path, so the aliased file resolves. The issue requests no automated test, and the change is a one-line configuration fix. The author's before and after build results are reported in the PR description. I did not run the build. The result rests on the code change and the correct decoding behavior of fileURLToPath.

Full details: Out of Scope Changes check

Explanation

The whole-PR diff changes one file, apps/web/vite.config.ts. It adds the node:url import and changes the fonts-CJK alias expression. Both edits serve the objective of #29587. No unrelated file or behavior changes appear in the diff. The node:url import follows the existing node:fs/promises and node:zlib imports in the same file. The raw summary reports no changes to exported declarations, which matches the diff.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
External image Recent review info

External image Run configuration

External image Commits

Reviewing files that changed from the base of the PR and between e68e19d605485db6ef31b2033f67a9d07634fc2b and 51ca6c0594a1e3b334e3aae7b2abf59b34136623.

External image Files selected for processing: 1

  • apps/web/vite.config.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.

Mmaria7d ago

Note

Written by gpt-6.1-sol on behalf of Maria

Verified in a real dev environment

verified exact head 51ca6c0594a1e3b334e3aae7b2abf59b34136623 on a linux boat vm. no source changes were needed.

  • reproduced on github/main (8b44f42b14): the web build in a real directory containing spaces failed with UNLOADABLE_DEPENDENCY for real%20t3%20main/.../fonts-cjk.ts.
  • the pr's web build passed in a real directory containing spaces and in a normal path. builds and check:changed ran through blacksmith. format, lint, knip, and affected typechecks passed.
  • ran vp i and the real background dev server from /home/user/pr space. used the repo's playwright with /usr/bin/google-chrome.
  • added a real git project through the app. a real codex gpt-6.1-sol turn created word, powerpoint, and markdown files. the word viewer rendered the heading, paragraph, bold, and italic text. the powerpoint viewer rendered its title and subtitle. markdown preview worked.
  • a second codex turn read the files and returned hashes matching an independent sha256sum check. no mocks, fixtures, or demo app state.

command transcript

word document rendered from the spaced checkout

powerpoint rendered from the spaced checkout

word, powerpoint, and markdown viewer interaction recording

Not verified

native desktop, ios, android, macos, and windows were not exercised on this linux vm. remote relay/tunnel connections and other providers were not exercised. optional cjk glyph coverage was not tested; this change preserves the existing font fallback.

Added 1 commit

  • Merge remote-tracking branch 'origin/main' into fix/web-build-path-with-spacee553f9f
JJulius Marminge20h ago

Evidence

In independent checkouts named checkout with spaces, run vp i at the root, then vp build from apps/web. Both installs succeeded. Environment for both captures: Linux x86_64, Node 26.12.1, pnpm 12.14.0, Vite+ 1.4.0; no seeded state. These images show excerpts of actual captured build output.

BeforeAfter
b3fdf2b10fe553f9f4dc
Before: build fails loading percent-encoded fonts-cjk pathAfter: build succeeds and emits fonts-cjk asset
Exit 1: UNLOADABLE_DEPENDENCY, checkout%20with%20spaces/.../fonts-cjk.ts, file not found.Exit 0: font asset emitted; production build completed.

Before full build transcript · After full build transcript · Reproduction driver

AI-assisted; verified with the tests and tools listed above.

JJulius Marminge4h ago

Thanks for this. Main has the change since #29051.

Compared the complete diff: this PR only corrects URL decoding for the fonts-cjk stub alias, while main removes that alias and stub entirely and resolves the installed font package, eliminating the same build failure.

Closing as superseded.