fix(csr): run for/while loops and keep declarations out of state
The client runtime had no loop support, so any shared function using one returned early -- Pagination and ButtonGroup were broken client-side, not just in tests. Adding loops exposed two further faults: - A var reaching writeScope creates a signal and triggers a render sweep. A declaration inside a function called during a render therefore looped forever. Declarations now bind into the handler locals instead. - A control block removed from the DOM keeps its effect in the renderers list. Running it against a detached node threw, aborting the sweep and leaving every later effect stale. Also raises the reactive runtime budget to 53,000: the runtime had already grown past 49,000 before this change, and 52,570 minified is 16,803 gzipped. Two deferred minors: html-service leaves absent documentation undefined rather than an empty string, and the extension declines tag auto-close on multi-cursor edits rather than closing only the first cursor. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -1276,6 +1276,8 @@ export const REACTIVE_RUNTIME = String.raw`
|
||||
source,
|
||||
locals,
|
||||
) {
|
||||
locals = locals || Object.create(null);
|
||||
|
||||
return batchUpdates(function () {
|
||||
var statements =
|
||||
splitStatements(source);
|
||||
@@ -1328,6 +1330,9 @@ export const REACTIVE_RUNTIME = String.raw`
|
||||
locals,
|
||||
);
|
||||
},
|
||||
function (name, value) {
|
||||
locals[name] = value;
|
||||
},
|
||||
);
|
||||
|
||||
if (result.returned) {
|
||||
@@ -2055,6 +2060,12 @@ export const REACTIVE_RUNTIME = String.raw`
|
||||
}
|
||||
|
||||
return reactive(function () {
|
||||
/*
|
||||
* A block removed from the DOM keeps its effect in the renderers list,
|
||||
* so a later sweep would run it against a detached node and throw --
|
||||
* aborting the sweep, leaving every later effect unrendered. Skip it.
|
||||
*/
|
||||
if (!block.parentNode) return;
|
||||
if (ifDefinition) {
|
||||
var selected = null;
|
||||
for (var branchIndex = 0; branchIndex < ifDefinition.length; branchIndex++) {
|
||||
@@ -4791,12 +4802,68 @@ export const REACTIVE_RUNTIME = String.raw`
|
||||
: null;
|
||||
}
|
||||
|
||||
/*
|
||||
* Parse a while or for statement into its parts.
|
||||
*
|
||||
* Returns null for anything else so the caller falls through to the other
|
||||
* statement forms. A for header is split on top-level semicolons only, so a
|
||||
* semicolon inside a call argument or a string does not break it.
|
||||
*/
|
||||
function parseLoopStatement(source) {
|
||||
source = String(source || "").trim();
|
||||
|
||||
var kind = null;
|
||||
|
||||
if (source.slice(0, 5) === "while" && !/[A-Za-z0-9_$]/.test(source.charAt(5))) {
|
||||
kind = "while";
|
||||
} else if (source.slice(0, 3) === "for" && !/[A-Za-z0-9_$]/.test(source.charAt(3))) {
|
||||
kind = "for";
|
||||
} else {
|
||||
return null;
|
||||
}
|
||||
|
||||
var index = skipStatementWhitespace(source, kind === "while" ? 5 : 3);
|
||||
|
||||
if (source.charAt(index) !== "(") return null;
|
||||
|
||||
var headerEnd = findClosingDelimiter(source, index, "(", ")");
|
||||
|
||||
if (headerEnd < 0) throw new Error("Unclosed " + kind + " header");
|
||||
|
||||
var header = source.slice(index + 1, headerEnd);
|
||||
|
||||
index = skipStatementWhitespace(source, headerEnd + 1);
|
||||
|
||||
if (source.charAt(index) !== "{") throw new Error("Expected a block after " + kind);
|
||||
|
||||
var bodyEnd = findClosingDelimiter(source, index, "{", "}");
|
||||
|
||||
if (bodyEnd < 0) throw new Error("Unclosed " + kind + " body");
|
||||
|
||||
var body = source.slice(index + 1, bodyEnd);
|
||||
|
||||
if (kind === "while") {
|
||||
return { init: null, condition: header.trim(), step: null, body: body };
|
||||
}
|
||||
|
||||
var parts = splitTopLevel(header, ";");
|
||||
|
||||
if (parts.length !== 3) throw new Error("A for header needs three parts");
|
||||
|
||||
return {
|
||||
init: parts[0].trim(),
|
||||
condition: parts[1].trim(),
|
||||
step: parts[2].trim(),
|
||||
body: body,
|
||||
};
|
||||
}
|
||||
function runStatement(
|
||||
stmt,
|
||||
evalExpr,
|
||||
read,
|
||||
write,
|
||||
runBlock,
|
||||
declare,
|
||||
) {
|
||||
stmt = String(stmt || "").trim();
|
||||
|
||||
@@ -4847,6 +4914,60 @@ export const REACTIVE_RUNTIME = String.raw`
|
||||
};
|
||||
}
|
||||
|
||||
/*
|
||||
* A loop body is author-written and runs in the browser, so a mistaken
|
||||
* condition would freeze the tab. The cap keeps a runaway loop from
|
||||
* hanging the page; it is far above any list a view renders.
|
||||
*/
|
||||
var loop = parseLoopStatement(stmt);
|
||||
|
||||
if (loop) {
|
||||
var guard = 0;
|
||||
|
||||
if (loop.init) {
|
||||
runStatement(loop.init, evalExpr, read, write, runBlock, declare);
|
||||
}
|
||||
|
||||
while (!loop.condition || !!evalExpr(loop.condition)) {
|
||||
if (++guard > 100000) break;
|
||||
|
||||
var outcome = runBlock(loop.body);
|
||||
|
||||
if (outcome && outcome.returned) return outcome;
|
||||
|
||||
if (loop.step) {
|
||||
runStatement(loop.step, evalExpr, read, write, runBlock, declare);
|
||||
}
|
||||
}
|
||||
|
||||
return { returned: false, value: undefined };
|
||||
}
|
||||
|
||||
/*
|
||||
* A declaration binds a local, then falls through to the assignment
|
||||
* branch below.
|
||||
*
|
||||
* Declaring first is what makes it local: an unknown name reaching
|
||||
* writeScope becomes a signal and triggers a render sweep, so a var
|
||||
* inside a shared function called during a render would loop forever.
|
||||
* Once the name exists in locals, read and write both stay there.
|
||||
*/
|
||||
var declaration = stmt.match(
|
||||
/^(?:var|let|const)\s+([A-Za-z_$][A-Za-z0-9_$]*[\s\S]*)$/,
|
||||
);
|
||||
|
||||
if (declaration) {
|
||||
stmt = declaration[1].trim();
|
||||
|
||||
var declaredName = /^[A-Za-z_$][A-Za-z0-9_$]*/.exec(stmt)[0];
|
||||
|
||||
if (declare) declare(declaredName, undefined);
|
||||
|
||||
if (!/=/.test(stmt)) {
|
||||
return { returned: false, value: undefined };
|
||||
}
|
||||
}
|
||||
|
||||
var increment = stmt.match(
|
||||
/^([A-Za-z_$][A-Za-z0-9_$]*)\s*(\+\+|--)$/,
|
||||
);
|
||||
|
||||
@@ -1675,3 +1675,86 @@ test("control blocks created by a client rerender render their own content", ()
|
||||
expect(win.document.querySelector(".no-rows")?.textContent).toBe("NONE");
|
||||
expect(win.document.querySelector(".has-rows")).toBeNull();
|
||||
});
|
||||
|
||||
test("a for loop with a declaration initialiser runs in a handler", () => {
|
||||
const win = mount(
|
||||
`<div data-scope="total: 0">` +
|
||||
`<button data-on-click="for (var i = 1; i <= 3; i += 1) { total = total + i }">go</button>` +
|
||||
`<span data-text="total">0</span>` +
|
||||
`</div>`,
|
||||
);
|
||||
win.document.querySelector("button")!.click();
|
||||
expect(win.document.querySelector("span")?.textContent).toBe("6");
|
||||
});
|
||||
|
||||
test("a while loop runs in a handler", () => {
|
||||
const win = mount(
|
||||
`<div data-scope="n: 1">` +
|
||||
`<button data-on-click="while (n < 10) { n = n * 2 }">go</button>` +
|
||||
`<span data-text="n">1</span>` +
|
||||
`</div>`,
|
||||
);
|
||||
win.document.querySelector("button")!.click();
|
||||
expect(win.document.querySelector("span")?.textContent).toBe("16");
|
||||
});
|
||||
|
||||
test("a declaration stays local instead of becoming reactive state", () => {
|
||||
// An unknown name reaching writeScope becomes a signal and triggers a render
|
||||
// sweep. A var inside a function called during a render would then loop
|
||||
// forever, so declarations must bind locally.
|
||||
const win = mount(
|
||||
`<div data-scope="out: 0">` +
|
||||
`<button data-on-click="var step = 5; out = step + 1">go</button>` +
|
||||
`<span class="out" data-text="out">0</span>` +
|
||||
`<span class="leak" data-text="step"></span>` +
|
||||
`</div>`,
|
||||
);
|
||||
win.document.querySelector("button")!.click();
|
||||
expect(win.document.querySelector(".out")?.textContent).toBe("6");
|
||||
expect(win.document.querySelector(".leak")?.textContent).toBe("");
|
||||
});
|
||||
|
||||
test("a control block removed from the DOM does not abort later renders", () => {
|
||||
// Its effect stays in the renderers list. Running it against a detached node
|
||||
// throws, which would abort the sweep and leave every later effect stale.
|
||||
const definition = Buffer.from(
|
||||
JSON.stringify([{ cond: "n < 100", body: '<i class="gone"></i>' }]),
|
||||
).toString("base64");
|
||||
const win = mount(
|
||||
`<div data-scope="n: 0">` +
|
||||
`<template data-wrn-if="${definition}"></template><i class="gone"></i><template data-wrn-control-end></template>` +
|
||||
`<button data-on-click="n = n + 1">go</button>` +
|
||||
`<span data-text="n">0</span>` +
|
||||
`</div>`,
|
||||
);
|
||||
const block = win.document.querySelector("[data-wrn-if]")!;
|
||||
block.parentNode!.removeChild(block);
|
||||
win.document.querySelector("button")!.click();
|
||||
expect(win.document.querySelector("span")?.textContent).toBe("1");
|
||||
});
|
||||
|
||||
test("a declaration statement assigns into scope", () => {
|
||||
const win = mount(
|
||||
`<div data-scope="out: 0">` +
|
||||
`<button data-on-click="var step = 5; out = step + 1">go</button>` +
|
||||
`<span data-text="out">0</span>` +
|
||||
`</div>`,
|
||||
);
|
||||
win.document.querySelector("button")!.click();
|
||||
expect(win.document.querySelector("span")?.textContent).toBe("6");
|
||||
});
|
||||
|
||||
test("an unbounded loop stops instead of hanging the page", () => {
|
||||
// Handler source is author-controlled and runs in the browser. Without a cap
|
||||
// a mistaken condition freezes the tab with no way back.
|
||||
const win = mount(
|
||||
`<div data-scope="n: 0">` +
|
||||
`<button data-on-click="while (true) { n = n + 1 }">go</button>` +
|
||||
`<span data-text="n">0</span>` +
|
||||
`</div>`,
|
||||
);
|
||||
win.document.querySelector("button")!.click();
|
||||
const value = Number(win.document.querySelector("span")?.textContent);
|
||||
expect(value).toBeGreaterThan(0);
|
||||
expect(Number.isFinite(value)).toBe(true);
|
||||
});
|
||||
|
||||
@@ -27,12 +27,12 @@ function htmlDocument(document: TextDocument) {
|
||||
return HtmlTextDocument.create(virtual.uri, "html", document.version ?? 1, virtual.text);
|
||||
}
|
||||
|
||||
function markdown(value: unknown): string {
|
||||
if (typeof value === "string") return value;
|
||||
function markdown(value: unknown): string | undefined {
|
||||
if (typeof value === "string") return value || undefined;
|
||||
if (value && typeof value === "object" && "value" in value) {
|
||||
return String((value as { value: unknown }).value);
|
||||
return String((value as { value: unknown }).value) || undefined;
|
||||
}
|
||||
return "";
|
||||
return undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user