fix: repair broken CI and browser test toolchain#229
Open
LuLaValva wants to merge 8 commits into
Open
Conversation
Bring the CI matrix onto supported Node versions and modernize the browser test toolchain so the @marko/serve suite can run again. Upgrades puppeteer and webpack-dev-server, and fixes several latent build bugs that surfaced with the newer toolchain (server bundle codegen, browserslist config resolution, and static generation of param routes). Fixes #228
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe CI matrix now tests Node.js 18, 20, and 22 with newer GitHub Actions. Build generation adds statement terminators and preserves browserslist environment names from configuration files. Static prerendering shares a cache and excludes dynamic parameter routes unless explicit paths are provided. The serve package upgrades webpack development server dependencies and uses its asynchronous start API. Parameterized fixtures define explicit prerender paths, and tests use the server’s stop method. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
The @marko/test package declares a chromedriver: * peer dependency. npm 7+ auto-installs it, pulling chromedriver's latest whose postinstall crashes on Node 18+ (require of ESM-only proxy-agent), which broke the install step. Restore the original npm 6 behaviour of not auto-installing peers via .npmrc and regenerate the lockfile without chromedriver.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 17-18: Configure the actions/checkout@v4 step with
persist-credentials set to false so the GitHub token is not retained in local
Git configuration; only enable persistence if a later workflow step explicitly
requires authenticated Git access.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cb816ab8-c2c8-4ac5-ba77-cbd56b9d5476
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonand included by**yarn.lockis excluded by!**/yarn.lock,!**/*.lockand included by**
📒 Files selected for processing (9)
.github/workflows/ci.ymlpackage.jsonpackages/build/src/index.jspackages/build/src/util.jspackages/serve/package.jsonpackages/serve/src/index.jspackages/serve/test/fixtures/params-and-queries-deprecated/test.jspackages/serve/test/fixtures/params-and-queries/test.jspackages/serve/test/index.test.js
- Override the transitive cheerio dependency (via get-hrefs in @marko/build) to 1.0.0-rc.12 so it stops pulling undici. Newer undici requires Node 20+ and references the File global at load time, which crashed the serve suite on Node 18 with 'ReferenceError: File is not defined'. get-hrefs only uses cheerio's HTML parser, so the older line is functionally equivalent here. - Set persist-credentials: false on actions/checkout so the GitHub token is not left in local git config for repo-controlled npm scripts to read.
Regenerate the browser suite's expected HTML for the upgraded toolchain and update the static param-route snapshots to the now-correctly rendered pages (previously an empty body from the routing regression). The dev-server's split vendor chunk is auto-named after whichever module webpack happens to sort first, which differs between build environments (webpack-dev-server client vs mini-css-extract HMR runtime). Normalize that name in the snapshot comparison so the serve snapshots no longer depend on the environment they were generated in.
webpack-dev-server v5 enables hot module replacement by default, which patches modules in place without a full page reload. The serve dev flow (and its browser tests) rely on a full reload to reflect edits, since the server-rendered Marko output has no client runtime to hot-patch. With HMR on, editing a file never fired a browser load event, hanging the add-route and update test scenarios. Disable hot and keep liveReload to restore the previous v3 full-reload behaviour.
The deprecated codecov@3 client crashes on newer Node when it can't upload (TypeError: result.split is not a function), and PR branches have no upload token anyway. Coverage reporting is best-effort, so let the step fail without failing the whole job.
The prettyprint output for an escaped backslash before a placeholder
normalizes `\\${foo}` to `${foo}`; both forms render identically, so
the pretty-printed result is equivalent. Update the expected snapshot to
the current output. Surfaced now that CI can run the suite again.
LuLaValva
force-pushed
the
claude/github-issue-228-p7q1sb
branch
from
July 10, 2026 18:17
e553f8c to
0e8f2ec
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #229 +/- ##
==========================================
- Coverage 66.39% 65.04% -1.36%
==========================================
Files 62 62
Lines 1717 1722 +5
==========================================
- Hits 1140 1120 -20
- Misses 577 602 +25 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The repo shipped a yarn.lock but CI ran a bare npm install (no lockfile),
so installs were non-deterministic. Commit to npm properly:
- point lerna's npmClient at npm (was yarn) so publish/clean use npm
- run npm ci in CI for a deterministic install that also verifies the
committed package-lock.json is complete
- gitignore yarn.lock so it can't creep back in
Handle the two peer/resolution edge cases npm 7+ surfaces that yarn v1
did not (yarn never auto-installed peers and pinned older transitives):
- chromedriver: a required peer of the legacy wdio-chromedriver-service
whose latest postinstall does a top-level require('proxy-agent') that
throws ERR_REQUIRE_ESM on Node 18+. Pin it to the last CommonJS-safe
version and skip its unused binary download via .npmrc.
- cheerio (via get-hrefs in @marko/build): newer cheerio pulls undici,
which references the File global at load time and crashes the suite on
Node 18. Pin cheerio to the rc line that has no undici dependency.
LuLaValva
force-pushed
the
claude/github-issue-228-p7q1sb
branch
from
July 10, 2026 21:42
4441488 to
94946b4
Compare
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.
Description
Modernizes the CI setup and browser test toolchain so the
@marko/servesuite can run again.Motivation and Context
Fixes #228 — CI was pinned to unsupported Node versions and the browser test harness had bit-rotted, so the suite couldn't run.
Generated by Claude Code