fix: string-literal-aware comment stripper in build.js (#45, #38)#51
Merged
Conversation
Replace the regex-based comment stripper with a hand-written state-machine tokenizer in scripts/strip-comments.js. The old regex (/\/\/.*$/gm) would silently truncate any line containing a `//`, including URL string literals like "https://example.com". It also didn't handle /* block */ comments at all. The new stripper is aware of: - double- and single-quoted strings with backslash escapes - template literals with nested ${} interpolations - best-effort regex literal disambiguation - both // line comments and /* block */ comments outside strings New test file app/tests/test-comment-stripper.js (20 assertions) asserts URL preservation, string literal handling, escape sequences, block comments, and edge cases. Built app/index.html is byte-identical to the baseline because no current worker source hits the bug — this is a latent-defect fix verified by unit tests, not output diff. Closes #45 Closes #38 (build-script portion; Go portion addressed separately) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #45 (URL literal mangling) and the build-script portion of #38 (block comments ignored).
The old comment-stripper in
scripts/build.jswas a single regex:This silently truncated any line containing
//, including URL string literals like"https://example.com". It also did nothing about/* block */comments. Both are currently latent defects — no worker source hits them today — but would silently break the next time someone adds a URL literal.Change
scripts/strip-comments.js— hand-written state-machine tokenizer (~176 LOC with JSDoc). Handles:\escapes${...}interpolations (brace-depth tracked)// lineand/* block */comments outside strings/a\/b/doesn't crashscripts/build.jsswaps the regex call forstripComments(). No other changes.package.json.Tests
New file
app/tests/test-comment-stripper.js— 20 assertions covering URL preservation, all string-literal forms, escape sequences, block comments, and edge cases (unterminated strings, empty input).TDD cycle followed: test file written first, RED verified (MODULE_NOT_FOUND), GREEN verified after implementation.
Test plan
app/index.htmlbyte-identical to baseline (229229 bytes) — no real content was affected, correctness proven by unit tests