Conversation
|
There was a problem hiding this comment.
Pull request overview
This PR replaces the prior docs-size benchmark check (and its stored baselines) with a deterministic router SSG snapshot E2E test, and updates CI/scripts accordingly.
Changes:
- Removed the docs-size validation script + JSON baseline, and dropped the CI step that ran it.
- Added a Playwright E2E test that Vite-builds a dedicated router SSG fixture and compares normalized HTML + state dump against checked-in snapshots (with an update mode).
- Updated root
package.jsonscripts to add dedicated commands for running/updating the new router SSG snapshot test.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/validate-docs-size.ts | Deleted the standalone docs-size benchmark/diagnostic script. |
| scripts/docs-size-results.json | Deleted the stored docs-size baseline results. |
| .github/workflows/ci.yml | Removed the “Docs Size Benchmarks” CI step. |
| package.json | Removed docs-size benchmark scripts; added router SSG snapshot e2e run/update scripts. |
| e2e/qwik-e2e/tests/qwikrouter/ssg-snapshot.e2e.ts | New Playwright test that builds an SSG fixture and asserts normalized HTML/state snapshots. |
| e2e/qwik-e2e/apps/qwikrouter-ssg-snapshot/src/routes/index.tsx | New fixture route designed to produce stable, inspectable serialized state. |
| e2e/qwik-e2e/apps/qwikrouter-ssg-snapshot/src/root.tsx | New fixture root wiring RouterOutlet + canonical URL. |
| e2e/qwik-e2e/apps/qwikrouter-ssg-snapshot/src/entry.ssr.tsx | New SSR entry using createRenderer for the fixture. |
| e2e/qwik-e2e/apps/qwikrouter-ssg-snapshot/package.json | Declares the new fixture app as private ESM module. |
| e2e/qwik-e2e/apps/qwikrouter-ssg-snapshot/expected.ssg.html | Checked-in expected normalized SSG HTML snapshot. |
| e2e/qwik-e2e/apps/qwikrouter-ssg-snapshot/expected.state.txt | Checked-in expected normalized serialized-state dump snapshot. |
| scripts/submodule-core.ts | Removed importMetaFix prepend logic from core build outputs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }); | ||
| code = result.code!; | ||
|
|
||
| await writeFile(outPath, importMetaFix + code + '\n'); | ||
| await writeFile(outPath, code + '\n'); | ||
|
|
There was a problem hiding this comment.
importMetaFix was removed, so the generated core.mjs/core.prod.mjs no longer ensures import.meta.env exists at runtime. The core source still performs direct reads like import.meta.env.DEV/import.meta.env.TEST (no optional chaining), which will throw in Node/other non-Vite runtimes where import.meta.env is undefined. Please restore a runtime-safe initialization (or replace these reads during bundling) so the distributed core modules can be imported outside Vite without crashing.
scripts/submodule-core.ts
Outdated
| await Promise.all([esm]); | ||
|
|
There was a problem hiding this comment.
In submoduleCoreDev, const esm = await build(...) already awaits the build; await Promise.all([esm]) is redundant (and misleading since esm is the resolved build result, not a promise). Consider reverting to a single await build(...) for clarity.
| await Promise.all([esm]); |
built with Refined Cloudflare Pages Action⚡ Cloudflare Pages Deployment
|
No description provided.