diff --git a/.changeset/happy-cycles-smash.md b/.changeset/happy-cycles-smash.md new file mode 100644 index 0000000000..6fa6c4e8f3 --- /dev/null +++ b/.changeset/happy-cycles-smash.md @@ -0,0 +1,5 @@ +--- +"@marko/runtime-tags": patch +--- + +Fix issue where streams that had aborted but then finished were swallowing the abort error. diff --git a/.changeset/tame-balloons-turn.md b/.changeset/tame-balloons-turn.md new file mode 100644 index 0000000000..df5112c2e5 --- /dev/null +++ b/.changeset/tame-balloons-turn.md @@ -0,0 +1,5 @@ +--- +"@marko/runtime-tags": patch +--- + +Add errors with debug info when attempting to serialize a non serializable value. diff --git a/.changeset/thin-vans-fail.md b/.changeset/thin-vans-fail.md new file mode 100644 index 0000000000..a402001473 --- /dev/null +++ b/.changeset/thin-vans-fail.md @@ -0,0 +1,8 @@ +--- +"marko": patch +"@marko/runtime-tags": patch +"@marko/compiler": patch +"@marko/translator-interop-class-tags": patch +--- + +Ensure Marko 5 renderBodies are serialized properly across the compat layer. diff --git a/.sizes.json b/.sizes.json index e3e19a04ab..bea2028700 100644 --- a/.sizes.json +++ b/.sizes.json @@ -7,8 +7,8 @@ { "name": "*", "total": { - "min": 18072, - "brotli": 6619 + "min": 18085, + "brotli": 6624 } }, { diff --git a/.sizes/dom.js b/.sizes/dom.js index fca6bca737..8ebaa48269 100644 --- a/.sizes/dom.js +++ b/.sizes/dom.js @@ -1,4 +1,4 @@ -// size: 18072 (min) 6619 (brotli) +// size: 18085 (min) 6624 (brotli) var empty = [], rest = Symbol(); function attrTag(attrs2) { @@ -1535,10 +1535,11 @@ var classIdToBranch = new Map(), (conditionalOnlyChild = fn(conditionalOnlyChild)); }, queueEffect: queueEffect, - init() { + init(warp10Noop) { register("$C_s", (branch) => { classIdToBranch.set(branch.m5c, branch); - }); + }), + register("$C_b", warp10Noop); }, registerRenderer(fn) { register("$C_r", fn); diff --git a/packages/runtime-class/src/runtime/helpers/tags-compat/runtime-dom.js b/packages/runtime-class/src/runtime/helpers/tags-compat/runtime-dom.js index 38185cb404..cff5cd4f55 100644 --- a/packages/runtime-class/src/runtime/helpers/tags-compat/runtime-dom.js +++ b/packages/runtime-class/src/runtime/helpers/tags-compat/runtime-dom.js @@ -144,7 +144,7 @@ exports.p = function (domCompat) { } domCompat.registerRenderer(create5to6Renderer); - domCompat.init(); + domCompat.init(require("../serialize-noop").___noop); function renderAndMorph(scope, renderer, renderBody, input) { const out = defaultCreateOut(); diff --git a/packages/runtime-class/src/runtime/helpers/tags-compat/runtime-html.js b/packages/runtime-class/src/runtime/helpers/tags-compat/runtime-html.js index d362438f58..00c044ea2a 100644 --- a/packages/runtime-class/src/runtime/helpers/tags-compat/runtime-html.js +++ b/packages/runtime-class/src/runtime/helpers/tags-compat/runtime-html.js @@ -75,6 +75,9 @@ exports.p = function (htmlCompat) { tag.renderer; const renderBody5 = tag.renderBody || tag; + if (!renderer5 && renderBody5) { + htmlCompat.registerRenderBody(renderBody5); + } return (input, ...args) => { const out = defaultCreateOut(); let customEvents; diff --git a/packages/runtime-tags/src/__tests__/fixtures/unserializable-warning/__snapshots__/html.expected/template.js b/packages/runtime-tags/src/__tests__/fixtures/unserializable-warning/__snapshots__/html.expected/template.js new file mode 100644 index 0000000000..bd5d8cb2ee --- /dev/null +++ b/packages/runtime-tags/src/__tests__/fixtures/unserializable-warning/__snapshots__/html.expected/template.js @@ -0,0 +1,34 @@ +import * as _$ from "@marko/runtime-tags/debug/html"; +const _renderer = /* @__PURE__ */_$.createRenderer((input, _tagVar) => { + const _scope0_id = _$.nextScopeId(); + const Foo = { + content: _$.register(/* @__PURE__ */_$.createRenderer(() => { + const _scope1_id = _$.nextScopeId(); + const unserializable = { + nested: { + thing: Buffer.from("") + } + }; + const test = _$.register(function () { + return unserializable; + }, "__tests__/template.marko_1/test", _scope1_id); + _$.writeEffect(_scope1_id, "__tests__/template.marko_1_test"); + _$.debug(_$.writeScope(_scope1_id, { + "unserializable": unserializable, + "test": test + }), "__tests__/template.marko", "1:2", { + "unserializable": "2:10", + "test": "7:10" + }); + }), "__tests__/template.marko_1_renderer", _scope0_id) + }; + const _dynamicScope = _$.peekNextScope(); + _$.dynamicTagInput(_scope0_id, "#text/0", Foo, {}); + _$.debug(_$.writeScope(_scope0_id, { + "#text/0!": _$.writeExistingScope(_dynamicScope), + "#text/0(": _$.normalizeDynamicRenderer(Foo) + }), "__tests__/template.marko", 0, { + "Foo": "1:9" + }); +}); +export default /* @__PURE__ */_$.createTemplate("__tests__/template.marko", _renderer); \ No newline at end of file diff --git a/packages/runtime-tags/src/__tests__/fixtures/unserializable-warning/__snapshots__/ssr-sanitized.expected.md b/packages/runtime-tags/src/__tests__/fixtures/unserializable-warning/__snapshots__/ssr-sanitized.expected.md new file mode 100644 index 0000000000..e69de29bb2 diff --git a/packages/runtime-tags/src/__tests__/fixtures/unserializable-warning/__snapshots__/ssr.expected.md b/packages/runtime-tags/src/__tests__/fixtures/unserializable-warning/__snapshots__/ssr.expected.md new file mode 100644 index 0000000000..f49afc5829 --- /dev/null +++ b/packages/runtime-tags/src/__tests__/fixtures/unserializable-warning/__snapshots__/ssr.expected.md @@ -0,0 +1,4 @@ +# Emit error +``` + TypeError: Unable to serialize "unserializable" in __tests__/template.marko:2:10 (reading nested.thing) +``` \ No newline at end of file diff --git a/packages/runtime-tags/src/__tests__/fixtures/unserializable-warning/template.marko b/packages/runtime-tags/src/__tests__/fixtures/unserializable-warning/template.marko new file mode 100644 index 0000000000..3dec713b25 --- /dev/null +++ b/packages/runtime-tags/src/__tests__/fixtures/unserializable-warning/template.marko @@ -0,0 +1,15 @@ + + + + + + + diff --git a/packages/runtime-tags/src/__tests__/fixtures/unserializable-warning/test.ts b/packages/runtime-tags/src/__tests__/fixtures/unserializable-warning/test.ts new file mode 100644 index 0000000000..7d0916d83c --- /dev/null +++ b/packages/runtime-tags/src/__tests__/fixtures/unserializable-warning/test.ts @@ -0,0 +1 @@ +export const skip_dom = true; diff --git a/packages/runtime-tags/src/common/compat-meta.ts b/packages/runtime-tags/src/common/compat-meta.ts index a30e35ff56..38d26ed217 100644 --- a/packages/runtime-tags/src/common/compat-meta.ts +++ b/packages/runtime-tags/src/common/compat-meta.ts @@ -1,3 +1,4 @@ const prefix = MARKO_DEBUG ? "$compat_" : "$C_"; export const RENDERER_REGISTER_ID = prefix + (MARKO_DEBUG ? "renderer" : "r"); export const SET_SCOPE_REGISTER_ID = prefix + (MARKO_DEBUG ? "setScope" : "s"); +export const RENDER_BODY_ID = prefix + (MARKO_DEBUG ? "renderBody" : "b"); diff --git a/packages/runtime-tags/src/dom/compat.ts b/packages/runtime-tags/src/dom/compat.ts index ecf6cb37f5..def2302346 100644 --- a/packages/runtime-tags/src/dom/compat.ts +++ b/packages/runtime-tags/src/dom/compat.ts @@ -1,4 +1,5 @@ import { + RENDER_BODY_ID, RENDERER_REGISTER_ID, SET_SCOPE_REGISTER_ID, } from "../common/compat-meta"; @@ -15,10 +16,12 @@ const classIdToBranch = new Map(); export const compat = { patchConditionals, queueEffect, - init() { + init(warp10Noop: any) { register(SET_SCOPE_REGISTER_ID, (branch: BranchScope & { m5c: string }) => { classIdToBranch.set(branch.m5c, branch); }); + + register(RENDER_BODY_ID, warp10Noop); }, registerRenderer(fn: any) { register(RENDERER_REGISTER_ID, fn); diff --git a/packages/runtime-tags/src/html.ts b/packages/runtime-tags/src/html.ts index bc346ac243..0ff7fcd3b7 100644 --- a/packages/runtime-tags/src/html.ts +++ b/packages/runtime-tags/src/html.ts @@ -22,6 +22,7 @@ export { dynamicTagInput, } from "./html/dynamic-tag"; export { forIn, forInBy, forOf, forOfBy, forTo, forToBy } from "./html/for"; +export { debug } from "./html/serializer"; export { createTemplate } from "./html/template"; export { $global, diff --git a/packages/runtime-tags/src/html/compat.ts b/packages/runtime-tags/src/html/compat.ts index e1816f3f9f..2e34e61d88 100644 --- a/packages/runtime-tags/src/html/compat.ts +++ b/packages/runtime-tags/src/html/compat.ts @@ -1,4 +1,5 @@ import { + RENDER_BODY_ID, RENDERER_REGISTER_ID, SET_SCOPE_REGISTER_ID, } from "../common/compat-meta"; @@ -104,11 +105,16 @@ export const compat = { asyncOut.error(boundary.signal.reason); } else { queueMicrotask(() => { - const { scripts, html } = (head = prepareChunk(head)); - asyncOut.script(scripts); - asyncOut.write(html); - asyncOut.end(); - head.html = head.scripts = ""; + head = prepareChunk(head); + // `prepareChunk` will call the serializer which could + // have new promises, the boundary may no longer be `done`. + if (boundary.done) { + const { scripts, html } = head; + asyncOut.script(scripts); + asyncOut.write(html); + asyncOut.end(); + head.html = head.scripts = ""; + } }); } } @@ -121,4 +127,7 @@ export const compat = { register(id, () => {}), ); }, + registerRenderBody(fn: any) { + register(RENDER_BODY_ID, fn); + }, }; diff --git a/packages/runtime-tags/src/html/serializer.ts b/packages/runtime-tags/src/html/serializer.ts index 5705e933e5..508b642d15 100644 --- a/packages/runtime-tags/src/html/serializer.ts +++ b/packages/runtime-tags/src/html/serializer.ts @@ -272,6 +272,7 @@ class State { } class Reference { + declare debug?: Debug; public init = ""; public assigns = ""; constructor( @@ -289,6 +290,29 @@ class Reference { } } +interface Debug { + file: string; + loc: string | 0; + vars: Record | undefined; +} +const DEBUG = new WeakMap(); +export function debug( + obj: WeakKey, + file: string, + loc: string | 0, + vars?: Record, +) { + if (MARKO_DEBUG) { + DEBUG.set(obj, { + file, + loc, + vars, + }); + } + + return obj; +} + export class Serializer { #state = new State(); get flushed() { @@ -420,6 +444,7 @@ function writeProp( return writeObject(state, val, parent, accessor); default: + MARKO_DEBUG && throwUnserializable(state, val, parent, accessor); return false; } } @@ -459,6 +484,11 @@ function writeReferenceOr( val, (ref = new Reference(parent, accessor, state.flush, state.buf.length)), ); + + if (MARKO_DEBUG) { + ref.debug = DEBUG.get(val); + } + if (write(state, val, ref)) return true; state.refs.delete(ref); @@ -567,7 +597,8 @@ function writeUnknownSymbol(state: State) { return true; } -function writeNever() { +function writeNever(state: State, val: unknown, ref: Reference) { + MARKO_DEBUG && throwUnserializable(state, val, ref); return false; } @@ -658,6 +689,8 @@ function writeUnknownObject(state: State, val: object, ref: Reference) { case globalThis.Response: return writeResponse(state, val as Response, ref); } + + MARKO_DEBUG && throwUnserializable(state, val, ref); return false; } @@ -1198,6 +1231,51 @@ function writeAsyncCall( boundary.endAsync(); } +function throwUnserializable( + state: State, + cause: unknown, + ref: Reference | null = null, + accessor: string = "", +) { + if (cause !== undefined && state.boundary?.abort) { + let message = "Unable to serialize"; + let access = ""; + while (ref?.accessor) { + const debug = ref.parent?.debug; + if (debug) { + const varLoc = debug.vars?.[ref.accessor]; + if (varLoc) { + message += ` "${ref.accessor}" in ${debug.file}:${varLoc}`; + } else { + message += ` ${JSON.stringify(ref.accessor)} in ${debug.file}`; + if (debug.loc) { + message += `:${debug.loc}`; + } + } + break; + } + access = toAccess(ref.accessor) + access; + ref = ref.parent; + } + + if (accessor) { + access = toAccess(accessor) + access; + } + + if (access[0] === ".") { + access = access.slice(1); + } + + if (access) { + message += ` (reading ${access})`; + } + + const err = new TypeError(message, { cause }); + err.stack = undefined; + state.boundary.abort(err); + } +} + function isCircular( parent: Reference | null, ref: Reference, diff --git a/packages/runtime-tags/src/html/template.ts b/packages/runtime-tags/src/html/template.ts index 8824dec1d9..4e9d451a3b 100644 --- a/packages/runtime-tags/src/html/template.ts +++ b/packages/runtime-tags/src/html/template.ts @@ -91,23 +91,24 @@ class ServerRenderResult implements RenderResult { done = true; if (resolve) { resolve({ value, done: !value }); + value = ""; } }, ); return { next() { - if (value) { + if (aborted) { + return Promise.reject(reason); + } else if (value) { const result = { value, done: false }; value = ""; return Promise.resolve(result); + } else if (done) { + return Promise.resolve({ value: "", done }); + } else { + return new Promise(exec); } - - return done - ? Promise.resolve({ value: "", done }) - : aborted - ? Promise.reject(reason) - : new Promise(exec); }, throw(error: unknown) { if (!(done || aborted)) { @@ -221,12 +222,15 @@ class ServerRenderResult implements RenderResult { const { boundary } = head; (boundary.onNext = () => { - if (boundary.done) { - if (boundary.signal.aborted) { - reject(boundary.signal.reason); - } else { - head = prepareChunk(head); - if (boundary.done) resolve(flushChunk(head, true)); + if (boundary.signal.aborted) { + boundary.onNext = NOOP; + reject(boundary.signal.reason); + } else if (boundary.done) { + head = prepareChunk(head); + // `prepareChunk` will call the serializer which could + // have new promises, the boundary may no longer be `done`. + if (boundary.done) { + resolve(flushChunk(head, true)); } } })(); @@ -249,28 +253,30 @@ class ServerRenderResult implements RenderResult { const { boundary } = head; const onNext = (boundary.onNext = (write?: boolean) => { - if (write || boundary.done) { - if (boundary.signal.aborted) { - if (!tick) offTick(onNext); - onAbort(boundary.signal.reason); - return; + if (boundary.signal.aborted) { + if (!tick) offTick(onNext); + boundary.onNext = NOOP; + onAbort(boundary.signal.reason); + } else { + if (write || boundary.done) { + head = prepareChunk(head); } - head = prepareChunk(head); - } - - if (write || boundary.done) { - const html = flushChunk(head, boundary.done); - if (html) onWrite(html); - if (boundary.done) { - if (!tick) offTick(onNext); - onClose(); - } else { - tick = true; + // `prepareChunk` will call the serializer which could + // have new promises, the boundary may no longer be `done`. + if (write || boundary.done) { + const html = flushChunk(head, boundary.done); + if (html) onWrite(html); + if (boundary.done) { + if (!tick) offTick(onNext); + onClose(); + } else { + tick = true; + } + } else if (tick) { + tick = false; + queueTick(onNext); } - } else if (tick) { - tick = false; - queueTick(onNext); } }); @@ -286,3 +292,5 @@ class ServerRenderResult implements RenderResult { return flushChunk(prepareChunk(head), true); } } + +function NOOP() {} diff --git a/packages/runtime-tags/src/translator/util/references.ts b/packages/runtime-tags/src/translator/util/references.ts index c5df369908..9aee5ebb54 100644 --- a/packages/runtime-tags/src/translator/util/references.ts +++ b/packages/runtime-tags/src/translator/util/references.ts @@ -37,6 +37,7 @@ export type Binding = { id: number; name: string; type: BindingType; + loc: t.SourceLocation | null; section: Section; serialize: boolean | Set; aliases: Set; @@ -93,6 +94,7 @@ export function createBinding( upstreamAlias?: Binding["upstreamAlias"], upstreamExpression?: Binding["upstreamExpression"], property?: string, + loc: t.SourceLocation | null = null, declared = false, ): Binding { const id = getNextBindingId(); @@ -100,6 +102,7 @@ export function createBinding( id, name, type, + loc, section, property, declared, @@ -256,9 +259,9 @@ function createBindingsAndTrackReferences( type: BindingType, scope: t.Scope, section: Section, - upstreamAlias?: Binding["upstreamAlias"], - upstreamExpression?: Binding["upstreamExpression"], - property?: string, + upstreamAlias: Binding["upstreamAlias"] | undefined, + upstreamExpression: Binding["upstreamExpression"] | undefined, + property: string | undefined, ) { switch (lVal.type) { case "Identifier": @@ -269,6 +272,7 @@ function createBindingsAndTrackReferences( upstreamAlias, upstreamExpression, property, + lVal.loc, true, ); trackReferencesForBinding(scope.getBinding(lVal.name)!); @@ -285,6 +289,7 @@ function createBindingsAndTrackReferences( upstreamAlias, undefined, property, + lVal.loc, )); for (const prop of lVal.properties) { @@ -337,6 +342,7 @@ function createBindingsAndTrackReferences( upstreamAlias, undefined, property, + lVal.loc, )); let i = -1; diff --git a/packages/runtime-tags/src/translator/util/sections.ts b/packages/runtime-tags/src/translator/util/sections.ts index 21a4973a44..8d4ef56144 100644 --- a/packages/runtime-tags/src/translator/util/sections.ts +++ b/packages/runtime-tags/src/translator/util/sections.ts @@ -24,6 +24,7 @@ export enum ContentType { export interface Section { id: number; name: string; + loc: t.SourceLocation | undefined; depth: number; parent: Section | undefined; params: undefined | Binding; @@ -79,6 +80,7 @@ export function startSection( section = extra.section = { id: sections.length, name: sectionName, + loc: sectionNamePath?.node.loc || undefined, depth: parentSection ? parentSection.depth + 1 : 0, parent: parentSection, params: undefined, diff --git a/packages/runtime-tags/src/translator/util/signals.ts b/packages/runtime-tags/src/translator/util/signals.ts index ef6dccd714..e224acec89 100644 --- a/packages/runtime-tags/src/translator/util/signals.ts +++ b/packages/runtime-tags/src/translator/util/signals.ts @@ -12,7 +12,7 @@ import { import { forEachIdentifier } from "./for-each-identifier"; import { getDeclaredBindingExpression } from "./get-defined-binding-expression"; import { isStatefulReferences } from "./is-stateful"; -import { isOutputHTML } from "./marko-config"; +import { isOptimize, isOutputHTML } from "./marko-config"; import { find, forEach, type Opt, push } from "./optional"; import { type Binding, @@ -1017,16 +1017,40 @@ export function writeHTMLResumeStatements( } if (serializedProperties.length || forceResumeScope(section)) { - path.pushContainer( - "body", - t.expressionStatement( - callRuntime( - "writeScope", - scopeIdIdentifier, - t.objectExpression(serializedProperties), - ), - ), + let writeScope = callRuntime( + "writeScope", + scopeIdIdentifier, + t.objectExpression(serializedProperties), ); + + if (!isOptimize()) { + let debugVars: t.ObjectProperty[] | undefined; + forEach(section.bindings, (binding) => { + if (binding.loc) { + (debugVars ||= []).push( + t.objectProperty( + getScopeAccessorLiteral(binding), + t.stringLiteral( + `${binding.loc.start.line}:${binding.loc.start.column + 1}`, + ), + ), + ); + } + }); + + writeScope = callRuntime( + "debug", + writeScope, + t.stringLiteral(path.hub.file.opts.filenameRelative as string), + section.loc && section.loc.start.line != null + ? t.stringLiteral( + `${section.loc.start.line}:${section.loc.start.column + 1}`, + ) + : t.numericLiteral(0), + debugVars && t.objectExpression(debugVars), + ); + } + path.pushContainer("body", t.expressionStatement(writeScope)); } const resumeClosestBranch = diff --git a/packages/runtime-tags/src/translator/visitors/function.ts b/packages/runtime-tags/src/translator/visitors/function.ts index 28f780aa80..3da528402d 100644 --- a/packages/runtime-tags/src/translator/visitors/function.ts +++ b/packages/runtime-tags/src/translator/visitors/function.ts @@ -55,7 +55,9 @@ export default { : markoRoot.node.name : t.isVariableDeclarator(fn.parent) && t.isIdentifier(fn.parent.id) ? fn.parent.id.name - : "anonymous")); + : t.isObjectMethod(node) && t.isIdentifier(node.key) + ? node.key.name + : "anonymous")); if ( isMarkoAttribute(markoRoot) && @@ -103,12 +105,14 @@ function isFunction( ): fn is | t.NodePath | t.NodePath - | t.NodePath { + | t.NodePath + | t.NodePath { switch (fn.node.type) { case "FunctionDeclaration": return isStatic && !fn.node.declare; case "FunctionExpression": case "ArrowFunctionExpression": + case "ObjectMethod": return true; default: return false;