Uh oh!
There was an error while loading. Please reload this page.
Keep published runtime bundles ASCII-only - #239
Conversation
🦋 Changeset detectedLatest commit: df539d2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
A typographic minus in a websocket.ts doc comment (introduced in cloudflare#209) survived bundling into every dist runtime bundle in 0.11.0. Consumers that inline the bundle through Latin-1-only APIs like btoa() throw on any character above U+00FF and silently corrupt anything above U+007F. Fix the comment, and enforce the invariant where it actually holds: a post-build check fails if any non-ASCII byte reaches a runtime bundle in dist/, since which source comments survive bundling is bundler implementation detail. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
a8e4e4a to
9a92ac7CompareUh oh!
There was an error while loading. Please reload this page.
teamchong
commented
Aug 12, 2026
/bonk review this |
There was a problem hiding this comment.
Reviewed. This is a clean, well-targeted fix and the enforcement approach (checking built output rather than source) is the right call — bundlers decide which comments survive, so guarding dist/ is the correct invariant. The .map and .d.ts exclusions look intentional and correct. A few non-blocking observations below.
Overall LGTM.
Notes:
- The changeset attributes the byte to 0.11.0. Per git history the
§5.5doc comment was introduced in #209 (commit ee7ca6f), so 0.11.0 is accurate for this character. @ndisidore's point about an earlier version is about cfos jumping 0.8.0 → 0.11.0 catching it late — worth reconciling the wording but not blocking. - The check reads
dist/non-recursively, which matches the current flat tsdown output. If entry-splitting ever emits shared chunks into subdirs, the check would silently skip them. See inline note. - The comment says "non-ASCII bytes" but the regex matches by code point after UTF-8 decode. Functionally equivalent for the goal; minor wording.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Review posted successfully. Here's a summary of my findings on PR #239: Review SummaryVerdict: LGTM — clean, well-targeted fix. The PR fixes a U+2212 (typographic minus) and Strengths
Non-blocking notes (posted inline)
|
… attribution Scan dist/ recursively so shared chunks emitted into subdirectories (possible per the tsdown config's own note when entries bundle together) can't slip past the check; verified a planted subdir file fails it. Say "characters" rather than "bytes" since the regex matches code points after UTF-8 decode, and attribute the U+2212 to cloudflare#209, which first shipped in 0.11.0. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
0.11.0's dist bundles carry a U+2212 (typographic minus) from a
websocket.tsdoc comment (#209). Discovered in Cloudflare OS capnweb bump. Consumers that inline the bundle - e.g. into adata:URL viabtoa(), which only accepts Latin-1 - throw on it, and anything in U+0080–U+00FF would corrupt silently instead.This fixes the comment and adds a post-build check that fails if any non-ASCII byte reaches a runtime bundle in
dist/. Runtime strings that legitimately need non-ASCII can use\uescapes in source.