Conversation
…o legacy-builds. Not working so well by far...
Dev mode has so much just thought, need to clean up the code.
…rk metadata Original work by Tal500.
There was a problem hiding this comment.
5 issues found across 503 files
Confidence score: 2/5
- SSR error handling is currently broken because
packages/kit/src/runtime/server/page/render.jsstill pushesserialized.errorinto the hydrate array after removing the serialization logic, so clients never receive server errors. - Streaming data loaders in legacy environments will break since
packages/kit/src/runtime/client/client.jsnow emits NDJSON chunks without newline delimiters when falling back fromResponse, leaving parsers stuck. - Documentation tweaks in
documentation/legacy/PR_DRAFT.md(branch name and typo) and the stray JSDoc onpackages/kit/src/runtime/server/page/render.jsdon’t block merging but should be cleaned up for clarity. - Pay close attention to
packages/kit/src/runtime/server/page/render.js,packages/kit/src/runtime/client/client.js, anddocumentation/legacy/PR_DRAFT.md– error propagation, NDJSON parsing, and docs accuracy need fixes.
Note: This PR contains a large number of files. cubic only reviews up to 75 files per PR, so some files may not have been reviewed.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="documentation/legacy/PR_DRAFT.md">
<violation number="1" location="documentation/legacy/PR_DRAFT.md:10">
P3: Fix the typo by spelling “properties” correctly so the documentation is clear.</violation>
<violation number="2" location="documentation/legacy/PR_DRAFT.md:25">
P2: The branch creation instructions reference `origin/master`, but this repository’s default branch is `main`, so the command will fail. Update the command to track `origin/main`.</violation>
</file>
<file name="packages/kit/src/runtime/client/client.js">
<violation number="1" location="packages/kit/src/runtime/client/client.js:2930">
P2: The legacy Response fallback drops newline delimiters, so NDJSON chunks can no longer be parsed in environments without `Response.body`, breaking data loading there.</violation>
</file>
<file name="packages/kit/src/runtime/server/page/render.js">
<violation number="1" location="packages/kit/src/runtime/server/page/render.js:342">
P2: Stray JSDoc comment with `@param` annotations is attached to the `hydrate` array declaration, which doesn't take any parameters. This comment should be removed as it's misleading.</violation>
<violation number="2" location="packages/kit/src/runtime/server/page/render.js:373">
P0: Critical bug: The error serialization code was removed, but `serialized.error` is still used in the hydrate array. This means errors will never be properly passed to the client, breaking SSR error handling. The `if (error) { serialized.error = devalue.uneval(error); }` block needs to be restored.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| if (input_list.length === 0) { | ||
| return codeBlocks.join(separator); | ||
| } | ||
| // otherwise |
There was a problem hiding this comment.
P0: Critical bug: The error serialization code was removed, but serialized.error is still used in the hydrate array. This means errors will never be properly passed to the client, breaking SSR error handling. The if (error) { serialized.error = devalue.uneval(error); } block needs to be restored.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/kit/src/runtime/server/page/render.js, line 373:
<comment>Critical bug: The error serialization code was removed, but `serialized.error` is still used in the hydrate array. This means errors will never be properly passed to the client, breaking SSR error handling. The `if (error) { serialized.error = devalue.uneval(error); }` block needs to be restored.</comment>
<file context>
@@ -328,6 +330,250 @@ export async function render_response({
+ if (input_list.length === 0) {
+ return codeBlocks.join(separator);
+ }
+ // otherwise
+
+ return legacy_support_and_export_init
</file context>
| // otherwise | |
| if (error) { | |
| serialized.error = devalue.uneval(error); | |
| } |
|
|
||
| 1. **Create a Clean Branch**: | ||
| ```bash | ||
| git checkout -b fix/legacy-plugin-support origin/master |
There was a problem hiding this comment.
P2: The branch creation instructions reference origin/master, but this repository’s default branch is main, so the command will fail. Update the command to track origin/main.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At documentation/legacy/PR_DRAFT.md, line 25:
<comment>The branch creation instructions reference `origin/master`, but this repository’s default branch is `main`, so the command will fail. Update the command to track `origin/main`.</comment>
<file context>
@@ -0,0 +1,36 @@
+
+1. **Create a Clean Branch**:
+ ```bash
+ git checkout -b fix/legacy-plugin-support origin/master
+ ```
+
</file context>
| git checkout -b fix/legacy-plugin-support origin/master | |
| + git checkout -b fix/legacy-plugin-support origin/main |
| if (current_line >= text_lines.length) { | ||
| return { done: true, value: undefined }; | ||
| } else { | ||
| return { done: false, value: text_lines[current_line++] }; |
There was a problem hiding this comment.
P2: The legacy Response fallback drops newline delimiters, so NDJSON chunks can no longer be parsed in environments without Response.body, breaking data loading there.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/kit/src/runtime/client/client.js, line 2930:
<comment>The legacy Response fallback drops newline delimiters, so NDJSON chunks can no longer be parsed in environments without `Response.body`, breaking data loading there.</comment>
<file context>
@@ -2895,12 +2895,58 @@ async function load_data(url, invalid) {
+ if (current_line >= text_lines.length) {
+ return { done: true, value: undefined };
+ } else {
+ return { done: false, value: text_lines[current_line++] };
+ }
+ }
</file context>
|
|
||
| const modern_import_func_var_name = '__KIT_modern_import_func'; | ||
|
|
||
| /** |
There was a problem hiding this comment.
P2: Stray JSDoc comment with @param annotations is attached to the hydrate array declaration, which doesn't take any parameters. This comment should be removed as it's misleading.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/kit/src/runtime/server/page/render.js, line 342:
<comment>Stray JSDoc comment with `@param` annotations is attached to the `hydrate` array declaration, which doesn't take any parameters. This comment should be removed as it's misleading.</comment>
<file context>
@@ -328,6 +330,250 @@ export async function render_response({
+
+ const modern_import_func_var_name = '__KIT_modern_import_func';
+
+ /**
+ * Generate JS init code for the HTML entry page
+ * @param {boolean} legacy_support_and_export_init
</file context>
| This PR addresses build failures when using `@vitejs/plugin-legacy` with SvelteKit. | ||
|
|
||
| ### The Problem | ||
| When `@vitejs/plugin-legacy` is used, Vite produces an array of output bundles (one for modern, one for legacy) instead of a single output object. SvelteKit's `writeBundle` hook previously assumed a single output, causing `TypeError`s when accessing properies like `bundle.output`. |
There was a problem hiding this comment.
P3: Fix the typo by spelling “properties” correctly so the documentation is clear.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At documentation/legacy/PR_DRAFT.md, line 10:
<comment>Fix the typo by spelling “properties” correctly so the documentation is clear.</comment>
<file context>
@@ -0,0 +1,36 @@
+This PR addresses build failures when using `@vitejs/plugin-legacy` with SvelteKit.
+
+### The Problem
+When `@vitejs/plugin-legacy` is used, Vite produces an array of output bundles (one for modern, one for legacy) instead of a single output object. SvelteKit's `writeBundle` hook previously assumed a single output, causing `TypeError`s when accessing properies like `bundle.output`.
+
+Additionally, the legacy plugin may rename/remap entry chunks. SvelteKit was strictly looking for `start.js` (or `entry.js`), resulting in "Entry not found" errors when the manifest contained `entry-legacy.js` or similar variations.
</file context>
| When `@vitejs/plugin-legacy` is used, Vite produces an array of output bundles (one for modern, one for legacy) instead of a single output object. SvelteKit's `writeBundle` hook previously assumed a single output, causing `TypeError`s when accessing properies like `bundle.output`. | |
| +When `@vitejs/plugin-legacy` is used, Vite produces an array of output bundles (one for modern, one for legacy) instead of a single output object. SvelteKit's `writeBundle` hook previously assumed a single output, causing `TypeError`s when accessing properties like `bundle.output`. |
@cubic-dev-ai review this pull request
Summary by cubic
Add first-class legacy browser builds (IE11+) by auto-integrating @vitejs/plugin-legacy, emitting modern and legacy client bundles with polyfills and safe bootstrapping for nomodule browsers. The fork is published as @tg-svelte/kit and includes a legacy-friendly browserslist.
New Features
Bug Fixes
Written for commit 731ead3. Summary will update on new commits.