fix(marko-virtual): consolidate Marko e2e into one in-package app, fix test - #1243
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe PR consolidates Marko E2E coverage into one shared Playwright app, routes tests to dedicated example fixtures, removes per-example E2E configuration, adds incremental-build cleanup, updates related Nx and tracing configuration, and improves chat latest-message pinning. ChangesMarko E2E consolidation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Playwright
participant MarkoE2EApp
participant ExampleFixture
participant Virtualizer
Playwright->>MarkoE2EApp: start build and preview
Playwright->>MarkoE2EApp: navigate to dedicated route
MarkoE2EApp->>ExampleFixture: render route fixture
ExampleFixture->>Virtualizer: configure and render virtualized content
Virtualizer-->>Playwright: expose positioned items and state
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
View your CI Pipeline Execution ↗ for commit d18dab0
☁️ Nx Cloud last updated this comment at |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@packages/marko-virtual/e2e/app/e2e/chat.spec.ts`:
- Around line 52-74: Require multiple consecutive unchanged scrollTop samples in
both waitForScrollSettled helpers, in
packages/marko-virtual/e2e/app/e2e/chat.spec.ts lines 52-74 and
packages/marko-virtual/e2e/app/e2e/chat-pretext.spec.ts lines 52-74. Track the
stable-sample count and resolve only after the shared stability threshold is
reached, preserving the existing polling and timeout behavior.
In `@packages/react-virtual/playwright.config.ts`:
- Around line 3-8: Update the Playwright webServer configuration to disable
reuseExistingServer, or require an app-specific health check before reusing an
existing server; ensure the local test flow does not skip vite preview merely
because port 5273 responds and cannot silently target an unrelated application.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b0fcd085-be93-44aa-9c3f-2c5adc03745e
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (149)
.changeset/marko-e2e-consolidation.md.gitignoreexamples/marko/chat-pretext/.marko-run/routes.d.tsexamples/marko/chat-pretext/e2e/README.mdexamples/marko/chat-pretext/package.jsonexamples/marko/chat-pretext/playwright.config.tsexamples/marko/chat-pretext/tsconfig.jsonexamples/marko/chat/.marko-run/routes.d.tsexamples/marko/chat/e2e/README.mdexamples/marko/chat/package.jsonexamples/marko/chat/playwright.config.tsexamples/marko/chat/tsconfig.jsonexamples/marko/dynamic/.marko-run/routes.d.tsexamples/marko/dynamic/package.jsonexamples/marko/dynamic/playwright.config.tsexamples/marko/dynamic/tsconfig.jsonexamples/marko/fixed/.marko-run/routes.d.tsexamples/marko/fixed/package.jsonexamples/marko/fixed/playwright.config.tsexamples/marko/fixed/tsconfig.jsonexamples/marko/grid/.marko-run/routes.d.tsexamples/marko/grid/package.jsonexamples/marko/grid/playwright.config.tsexamples/marko/grid/tsconfig.jsonexamples/marko/infinite-scroll/.marko-run/routes.d.tsexamples/marko/infinite-scroll/package.jsonexamples/marko/infinite-scroll/playwright.config.tsexamples/marko/infinite-scroll/tsconfig.jsonexamples/marko/padding/.marko-run/routes.d.tsexamples/marko/padding/package.jsonexamples/marko/padding/playwright.config.tsexamples/marko/padding/tsconfig.jsonexamples/marko/pretext/.marko-run/routes.d.tsexamples/marko/pretext/package.jsonexamples/marko/pretext/playwright.config.tsexamples/marko/pretext/tsconfig.jsonexamples/marko/scroll-padding/.marko-run/routes.d.tsexamples/marko/scroll-padding/package.jsonexamples/marko/scroll-padding/playwright.config.tsexamples/marko/scroll-padding/tsconfig.jsonexamples/marko/smooth-scroll/.marko-run/routes.d.tsexamples/marko/smooth-scroll/package.jsonexamples/marko/smooth-scroll/playwright.config.tsexamples/marko/smooth-scroll/tsconfig.jsonexamples/marko/ssr-fetch/.marko-run/routes.d.tsexamples/marko/ssr-fetch/package.jsonexamples/marko/ssr-fetch/playwright.config.tsexamples/marko/ssr-fetch/tsconfig.jsonexamples/marko/ssr-restore/.marko-run/routes.d.tsexamples/marko/ssr-restore/package.jsonexamples/marko/ssr-restore/playwright.config.tsexamples/marko/ssr-restore/tsconfig.jsonexamples/marko/ssr-slice/.marko-run/routes.d.tsexamples/marko/ssr-slice/package.jsonexamples/marko/ssr-slice/playwright.config.tsexamples/marko/ssr-slice/tsconfig.jsonexamples/marko/ssr/.marko-run/routes.d.tsexamples/marko/ssr/package.jsonexamples/marko/ssr/playwright.config.tsexamples/marko/ssr/tsconfig.jsonexamples/marko/sticky/.marko-run/routes.d.tsexamples/marko/sticky/package.jsonexamples/marko/sticky/playwright.config.tsexamples/marko/sticky/tsconfig.jsonexamples/marko/table/.marko-run/routes.d.tsexamples/marko/table/package.jsonexamples/marko/table/playwright.config.tsexamples/marko/table/tsconfig.jsonexamples/marko/variable/.marko-run/routes.d.tsexamples/marko/variable/package.jsonexamples/marko/variable/playwright.config.tsexamples/marko/variable/tsconfig.jsonexamples/marko/window-ssr-slice/.marko-run/routes.d.tsexamples/marko/window-ssr-slice/package.jsonexamples/marko/window-ssr-slice/playwright.config.tsexamples/marko/window-ssr-slice/tsconfig.jsonexamples/marko/window/.marko-run/routes.d.tsexamples/marko/window/package.jsonexamples/marko/window/playwright.config.tsexamples/marko/window/tsconfig.jsonnx.jsonpackages/angular-virtual/package.jsonpackages/angular-virtual/playwright.config.tspackages/marko-virtual/e2e/app/README.mdpackages/marko-virtual/e2e/app/e2e/chat-pretext.spec.tspackages/marko-virtual/e2e/app/e2e/chat.spec.tspackages/marko-virtual/e2e/app/e2e/dynamic.spec.tspackages/marko-virtual/e2e/app/e2e/fixed.spec.tspackages/marko-virtual/e2e/app/e2e/grid.spec.tspackages/marko-virtual/e2e/app/e2e/infinite-scroll.spec.tspackages/marko-virtual/e2e/app/e2e/option-gates.spec.tspackages/marko-virtual/e2e/app/e2e/padding.spec.tspackages/marko-virtual/e2e/app/e2e/pretext.spec.tspackages/marko-virtual/e2e/app/e2e/scroll-padding.spec.tspackages/marko-virtual/e2e/app/e2e/smooth-scroll.spec.tspackages/marko-virtual/e2e/app/e2e/ssr-fetch.spec.tspackages/marko-virtual/e2e/app/e2e/ssr-restore.spec.tspackages/marko-virtual/e2e/app/e2e/ssr-slice.spec.tspackages/marko-virtual/e2e/app/e2e/ssr.spec.tspackages/marko-virtual/e2e/app/e2e/sticky.spec.tspackages/marko-virtual/e2e/app/e2e/table.spec.tspackages/marko-virtual/e2e/app/e2e/variable.spec.tspackages/marko-virtual/e2e/app/e2e/window-ssr-slice.spec.tspackages/marko-virtual/e2e/app/e2e/window.spec.tspackages/marko-virtual/e2e/app/marko.jsonpackages/marko-virtual/e2e/app/package.jsonpackages/marko-virtual/e2e/app/playwright.config.tspackages/marko-virtual/e2e/app/src/routes/+page.markopackages/marko-virtual/e2e/app/src/routes/api/reply/+handler.tspackages/marko-virtual/e2e/app/src/routes/cached/+page.markopackages/marko-virtual/e2e/app/src/routes/chat-pretext/+page.markopackages/marko-virtual/e2e/app/src/routes/chat/+page.markopackages/marko-virtual/e2e/app/src/routes/debug/+page.markopackages/marko-virtual/e2e/app/src/routes/dynamic/+page.markopackages/marko-virtual/e2e/app/src/routes/enabled/+page.markopackages/marko-virtual/e2e/app/src/routes/fixed/+page.markopackages/marko-virtual/e2e/app/src/routes/grid/+page.markopackages/marko-virtual/e2e/app/src/routes/infinite-scroll/+page.markopackages/marko-virtual/e2e/app/src/routes/lanes-mode/+page.markopackages/marko-virtual/e2e/app/src/routes/measure-element/+page.markopackages/marko-virtual/e2e/app/src/routes/padding/+page.markopackages/marko-virtual/e2e/app/src/routes/pretext/+page.markopackages/marko-virtual/e2e/app/src/routes/rtl/+page.markopackages/marko-virtual/e2e/app/src/routes/scroll-events/+page.markopackages/marko-virtual/e2e/app/src/routes/scroll-margin/+page.markopackages/marko-virtual/e2e/app/src/routes/scroll-padding/+page.markopackages/marko-virtual/e2e/app/src/routes/smooth-scroll/+page.markopackages/marko-virtual/e2e/app/src/routes/ssr-fetch/+page.markopackages/marko-virtual/e2e/app/src/routes/ssr-restore/+page.markopackages/marko-virtual/e2e/app/src/routes/ssr-slice/+page.markopackages/marko-virtual/e2e/app/src/routes/ssr/+page.markopackages/marko-virtual/e2e/app/src/routes/sticky/+page.markopackages/marko-virtual/e2e/app/src/routes/table/+page.markopackages/marko-virtual/e2e/app/src/routes/variable/+page.markopackages/marko-virtual/e2e/app/src/routes/window-example/+page.markopackages/marko-virtual/e2e/app/src/routes/window-horizontal/+page.markopackages/marko-virtual/e2e/app/src/routes/window-initial-offset/+page.markopackages/marko-virtual/e2e/app/src/routes/window-ssr-slice/+page.markopackages/marko-virtual/e2e/app/src/routes/window/+page.markopackages/marko-virtual/e2e/app/tsconfig.jsonpackages/marko-virtual/e2e/app/vite.config.tspackages/marko-virtual/e2e/option-gates/.marko-run/routes.d.tspackages/marko-virtual/e2e/option-gates/README.mdpackages/marko-virtual/e2e/option-gates/playwright.config.tspackages/marko-virtual/e2e/option-gates/vite.config.tspackages/marko-virtual/package.jsonpackages/marko-virtual/tests/options.test.tspackages/react-virtual/package.jsonpackages/react-virtual/playwright.config.ts
💤 Files with no reviewable changes (82)
- examples/marko/infinite-scroll/playwright.config.ts
- examples/marko/dynamic/playwright.config.ts
- examples/marko/dynamic/.marko-run/routes.d.ts
- examples/marko/chat/e2e/README.md
- examples/marko/pretext/playwright.config.ts
- examples/marko/ssr-fetch/playwright.config.ts
- examples/marko/pretext/package.json
- examples/marko/ssr-fetch/tsconfig.json
- packages/marko-virtual/e2e/option-gates/vite.config.ts
- examples/marko/ssr/tsconfig.json
- packages/marko-virtual/e2e/option-gates/README.md
- examples/marko/window/playwright.config.ts
- examples/marko/grid/playwright.config.ts
- examples/marko/chat-pretext/playwright.config.ts
- examples/marko/chat-pretext/.marko-run/routes.d.ts
- examples/marko/fixed/playwright.config.ts
- examples/marko/chat/tsconfig.json
- examples/marko/ssr-slice/.marko-run/routes.d.ts
- examples/marko/sticky/.marko-run/routes.d.ts
- examples/marko/padding/.marko-run/routes.d.ts
- examples/marko/table/playwright.config.ts
- examples/marko/padding/tsconfig.json
- examples/marko/scroll-padding/playwright.config.ts
- examples/marko/sticky/tsconfig.json
- examples/marko/padding/playwright.config.ts
- examples/marko/infinite-scroll/.marko-run/routes.d.ts
- examples/marko/window-ssr-slice/package.json
- examples/marko/window-ssr-slice/tsconfig.json
- examples/marko/window-ssr-slice/playwright.config.ts
- examples/marko/table/.marko-run/routes.d.ts
- examples/marko/chat/playwright.config.ts
- examples/marko/smooth-scroll/tsconfig.json
- examples/marko/variable/tsconfig.json
- examples/marko/scroll-padding/tsconfig.json
- examples/marko/window-ssr-slice/.marko-run/routes.d.ts
- examples/marko/chat-pretext/e2e/README.md
- examples/marko/variable/.marko-run/routes.d.ts
- examples/marko/ssr-fetch/.marko-run/routes.d.ts
- examples/marko/smooth-scroll/package.json
- examples/marko/ssr-restore/tsconfig.json
- examples/marko/ssr/.marko-run/routes.d.ts
- examples/marko/ssr-slice/playwright.config.ts
- examples/marko/ssr-restore/playwright.config.ts
- examples/marko/chat/package.json
- examples/marko/pretext/tsconfig.json
- examples/marko/window/package.json
- examples/marko/variable/playwright.config.ts
- packages/marko-virtual/e2e/option-gates/playwright.config.ts
- examples/marko/ssr-restore/package.json
- examples/marko/sticky/package.json
- examples/marko/fixed/.marko-run/routes.d.ts
- examples/marko/pretext/.marko-run/routes.d.ts
- examples/marko/ssr/playwright.config.ts
- examples/marko/chat-pretext/package.json
- examples/marko/ssr/package.json
- examples/marko/ssr-slice/package.json
- examples/marko/grid/tsconfig.json
- examples/marko/window/tsconfig.json
- examples/marko/table/package.json
- examples/marko/fixed/tsconfig.json
- examples/marko/table/tsconfig.json
- examples/marko/chat/.marko-run/routes.d.ts
- examples/marko/ssr-restore/.marko-run/routes.d.ts
- examples/marko/smooth-scroll/.marko-run/routes.d.ts
- examples/marko/ssr-fetch/package.json
- examples/marko/infinite-scroll/tsconfig.json
- examples/marko/smooth-scroll/playwright.config.ts
- examples/marko/scroll-padding/.marko-run/routes.d.ts
- examples/marko/window/.marko-run/routes.d.ts
- examples/marko/ssr-slice/tsconfig.json
- examples/marko/variable/package.json
- examples/marko/sticky/playwright.config.ts
- examples/marko/scroll-padding/package.json
- examples/marko/padding/package.json
- examples/marko/fixed/package.json
- examples/marko/infinite-scroll/package.json
- examples/marko/grid/.marko-run/routes.d.ts
- examples/marko/chat-pretext/tsconfig.json
- examples/marko/dynamic/tsconfig.json
- packages/marko-virtual/e2e/option-gates/.marko-run/routes.d.ts
- examples/marko/dynamic/package.json
- examples/marko/grid/package.json
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 `@examples/marko/chat-pretext/src/routes/`+page.marko:
- Around line 195-204: Update the pinPending handling around the visible
state.pinPending branch so it is cleared only after history loading has
completed; when atEnd is reached while prependHistory is still in flight, retain
pinPending and continue issuing v.scrollToEnd() until the delayed prepend/update
finishes, while preserving the existing convergence behavior afterward.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7ba4f64c-5318-4723-84ac-97745ac4deab
📒 Files selected for processing (1)
examples/marko/chat-pretext/src/routes/+page.marko
… harden scroll-settle waits.
KevinVandy
approved these changes
Jul 28, 2026
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
2 participants
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.
Problem
The Marko example e2e suites fail with
Timed out waiting 120000ms from config.webServer, which hides the real error. Two defects, both reproduced:1.
test:e2ehad no build ordering. nx.json defineddependsOn: ["^build"]fortest:eslint,test:lib,test:typesandbuild— but had notest:e2eentry at all.pnpm test:prrunstest:e2eandbuildas unordered peers, and the examples depend on@tanstack/marko-virtual, whosemarko.jsonpoints at./dist/tags— a build output. When e2e wins the race, the Marko compiler dies loading the taglib:Every page 500s, and Playwright reports only the webServer timeout. With the package built first, 19/19 suites passed — so the tests were fine, the ordering wasn't.
2. The tags build silently emitted nothing.
tsBuildInfoFilepointed insidedist— which nx caches as a build output and thefilesfield publishes to npm — and@marko/type-checkhardcodesincremental: true. Adistcarrying that file but notdist/tagsmakes every later build a no-op: exit 0, nothing emitted,nx buildreporting success while the package's tag entry point stays missing. This is how a tree gets into the state defect 1 then trips over.Change
One in-package app at
packages/marko-virtual/e2e/app(the formeroption-gatesapp, promoted), with one Playwright config, one server and one port — replacing 19 example-local suites, 19 dev servers, 19 ports and 19 copies of the config. Same shapereact-virtualuses. Vite aliases resolve the adapter and core to source, so the suite needs no build at all; verified with bothdistdirectories wiped.One deliberate departure from
react-virtual: the example fixtures import the example page rather than copying it —so the shipped examples remain what the suite covers, and a fixture can't drift from the example it tests. (
examples/react/*currently has no e2e coverage at all; this keeps Marko's.) Examples become plain examples: noe2e/, noplaywright.config.ts, no@playwright/test.All 82 tests preserved — 71 example + 11 option gates.
Production build, and two mis-tiered tests
The suite now serves a production build instead of the dev server, matching
react-virtualand the usual convention for library repos (framework repos like Vite, Next.js and SvelteKit test both modes because their dev pipeline is the product). Every route is compiled up front, so no test pays a cold Marko compile mid-assertion, and the suite exercises the production SSR/resume path.Two tests were asserting dev-only behavior. Neither was dropped:
debuggate asserted aconsole.infothat core compiles out underprocess.env.NODE_ENV !== 'production'— the wrong tier twice over, since a console assertion needs no browser. The log proof moved totests/options.test.ts(jsdom, where the branch is live); the browser gate now asserts the option doesn't disturb rendering.ssr-sliceno-JS test asserted the placeholder is hidden, which is only true in dev. In production it stays visible, so the test was documenting behavior users never see. Re-derived against production.A real flake
The
chatspecs captured a scroll baseline immediately after a programmatic jump, while the virtualizer was still compensating the offset from fresh measurements (~102px drift) — so "this append did not move the view" raced the settle. Added a scroll-settle wait to both chat specs. Same compensation path as a5417b4. Six consecutive clean runs afterwards.Also in here
trace: 'retain-on-failure'; react's port moved off vite's default 5173 (withreuseExistingServer, a stray dev server elsewhere in the tree made the suite pass against the wrong app) plus--strictPort;../../node_modules/.bin/playwrightreplaced with a bareplaywright(resolves via pnpm's script PATH)..marko-run/routes.d.tsuntracked and gitignored — it's generated bymarko-run dev|build, andmarko-type-checkpasses without it, including for the API-route examples.Verification
test:types,test:kniptest:libbuild,test:build,test:eslint,test:sherif,test:docsThe e2e suite was also run with
packages/marko-virtual/distandpackages/virtual-core/distdeleted, to confirm the no-build property holds.Notes for review
pr.yml. Happy to include it here if wanted.marko.jsontags-dirstill reaches into the packagesrc, so an example copied out of the monorepo (pkg-pr-new publishes./examples/*/*as templates) won't compile. That's a separate bug from the e2e setup.🤖 Generated with Claude Code
✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit