diff --git a/.changeset/fix-jsx-clean-teardown-race.md b/.changeset/fix-jsx-clean-teardown-race.md new file mode 100644 index 00000000..1479c19c --- /dev/null +++ b/.changeset/fix-jsx-clean-teardown-race.md @@ -0,0 +1,12 @@ +--- +"@tko/utils.jsx": patch +--- + +Add `options.jsxCleanBatchSize` (default `1000`) controlling JSX node cleanup +batching. Setting it to `0` runs cleanup synchronously on detach. Registered +via `defineOption` — the hardcoded `MAX_CLEAN_AT_ONCE` constant is gone, but +the new default matches it so production behavior is unchanged. + +Fixes a `ReferenceError: Element is not defined` in the `cli-happy-dom` test +project where the 25ms batch timer could fire after happy-dom had torn down +DOM globals. diff --git a/AGENTS.md b/AGENTS.md index e99b7811..98418830 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -105,6 +105,13 @@ packages/example/ Inter-package dependencies use `@tko/package-name` and are resolved via npm workspaces. +### Configurable runtime options (`ko.options.*`) + +Register package-owned options via `defineOption` from `@tko/utils`, not as +fields on the core `Options` class. See +[`tko.io/public/agents/options.md`](tko.io/public/agents/options.md) for the +pattern and canonical example. + ## CI/CD GitHub Actions workflows (`.github/workflows/`): diff --git a/builds/knockout/helpers/vitest-setup.js b/builds/knockout/helpers/vitest-setup.js index 2b2557d4..658e5d5d 100644 --- a/builds/knockout/helpers/vitest-setup.js +++ b/builds/knockout/helpers/vitest-setup.js @@ -1,5 +1,6 @@ import * as chai from 'chai' import sinon from 'sinon' +import { options } from '@tko/utils' import { isHappyDom } from '../../../packages/utils/helpers/test-env.ts' // Set globals that builds/knockout specs and mocha-test-helpers.js expect @@ -20,3 +21,11 @@ import '../dist/browser.min.js' // Now import the helper — it needs chai, expect, ko, and beforeEach/afterEach as globals. // beforeEach/afterEach come from vitest globals (globals: true in config). import './mocha-test-helpers.js' + +// Run JSX cleanup synchronously in tests. In happy-dom the 25ms batch +// timer can fire after the DOM globals are torn down, throwing from +// `cleanNode`. browser.min.js bundles its own Options instance, so mirror. +beforeAll(() => { + options.jsxCleanBatchSize = 0 + globalThis.ko.options.jsxCleanBatchSize = 0 +}) diff --git a/packages/utils.jsx/spec/jsxCleanBehaviors.ts b/packages/utils.jsx/spec/jsxCleanBehaviors.ts new file mode 100644 index 00000000..78474abf --- /dev/null +++ b/packages/utils.jsx/spec/jsxCleanBehaviors.ts @@ -0,0 +1,125 @@ +import { assert } from 'chai' +import sinon from 'sinon' + +import { addDisposeCallback, options } from '@tko/utils' + +import { queueCleanNode } from '../src/jsxClean' + +describe('jsxClean queue', function () { + let cleaned: Node[] + let clock: sinon.SinonFakeTimers + let originalBatchSize: number + + function makeNode(): HTMLElement { + const node = document.createElement('div') + addDisposeCallback(node, () => cleaned.push(node)) + return node + } + + beforeEach(function () { + cleaned = [] + originalBatchSize = options.jsxCleanBatchSize + clock = sinon.useFakeTimers() + }) + + afterEach(function () { + clock.restore() + options.jsxCleanBatchSize = originalBatchSize + }) + + describe('jsxCleanBatchSize = 0 (synchronous)', function () { + beforeEach(function () { + options.jsxCleanBatchSize = 0 + }) + + it('runs cleanup synchronously on queueCleanNode', function () { + const node = makeNode() + queueCleanNode(node) + assert.deepEqual(cleaned, [node]) + }) + + it('drains all queued nodes synchronously in a single call', function () { + const nodes = [makeNode(), makeNode(), makeNode()] + for (const n of nodes) queueCleanNode(n) + assert.deepEqual(cleaned, nodes) + }) + + it('does not schedule a timer', function () { + queueCleanNode(makeNode()) + assert.equal(clock.countTimers(), 0) + }) + }) + + describe('jsxCleanBatchSize > 0 (batched, default 1000)', function () { + beforeEach(function () { + options.jsxCleanBatchSize = 1000 + }) + + it('defers cleanup until the 25ms timer fires', function () { + const node = makeNode() + queueCleanNode(node) + assert.lengthOf(cleaned, 0, 'not cleaned before timer fires') + clock.tick(25) + assert.deepEqual(cleaned, [node]) + }) + + it('does not re-trigger the timer while one is already pending', function () { + const a = makeNode() + const b = makeNode() + queueCleanNode(a) + queueCleanNode(b) + clock.tick(24) + assert.lengthOf(cleaned, 0) + clock.tick(1) + assert.deepEqual(cleaned, [a, b]) + }) + + it('processes at most batchSize nodes per 25ms tick', function () { + options.jsxCleanBatchSize = 3 + const nodes = [makeNode(), makeNode(), makeNode(), makeNode(), makeNode()] + for (const n of nodes) queueCleanNode(n) + + clock.tick(25) + assert.lengthOf(cleaned, 3, 'first tick processes one batch') + + clock.tick(25) + assert.lengthOf(cleaned, 5, 'second tick drains the remainder') + }) + + it('re-schedules itself when the queue is non-empty after a batch', function () { + options.jsxCleanBatchSize = 2 + const nodes = Array.from({ length: 5 }, makeNode) + for (const n of nodes) queueCleanNode(n) + + clock.tick(25) + assert.lengthOf(cleaned, 2) + clock.tick(25) + assert.lengthOf(cleaned, 4) + clock.tick(25) + assert.lengthOf(cleaned, 5) + }) + + it('drains synchronously if batchSize is flipped to 0 while a timer is pending', function () { + const nodes = [makeNode(), makeNode(), makeNode()] + for (const n of nodes) queueCleanNode(n) + assert.equal(clock.countTimers(), 1) + + options.jsxCleanBatchSize = 0 + clock.tick(25) + + assert.deepEqual(cleaned, nodes) + assert.equal(clock.countTimers(), 0, 'no re-armed timer') + }) + + it('drains synchronously if batchSize becomes non-finite while a timer is pending', function () { + const nodes = [makeNode(), makeNode()] + for (const n of nodes) queueCleanNode(n) + + options.jsxCleanBatchSize = Number.NaN as unknown as number + clock.tick(25) + + assert.deepEqual(cleaned, nodes) + assert.equal(clock.countTimers(), 0) + }) + }) +}) diff --git a/packages/utils.jsx/src/jsxClean.ts b/packages/utils.jsx/src/jsxClean.ts index ca61253d..7e06b18f 100644 --- a/packages/utils.jsx/src/jsxClean.ts +++ b/packages/utils.jsx/src/jsxClean.ts @@ -1,26 +1,62 @@ -import { cleanNode } from '@tko/utils' +import { cleanNode, defineOption, options } from '@tko/utils' const DELAY_MS = 25 -const MAX_CLEAN_AT_ONCE = 1000 const cleanNodeQueue = new Array() let cleanNodeTimeoutID: ReturnType | null = null +// Extend the Options type so ko.options.jsxCleanBatchSize is strongly typed. +declare module '@tko/utils' { + interface Options { + jsxCleanBatchSize: number + } +} + +// `0` runs cleanup synchronously on detach. Test environments that tear +// down DOM globals between files use that to avoid a pending 25ms timer +// firing against a dead global. +defineOption('jsxCleanBatchSize', { default: 1000 }) + export function queueCleanNode(node) { cleanNodeQueue.push(node) - triggerCleanTimeout() + if (options.jsxCleanBatchSize === 0) { + flushAll() + } else { + scheduleBatch() + } } -function triggerCleanTimeout() { +function scheduleBatch() { if (!cleanNodeTimeoutID && cleanNodeQueue.length) { - cleanNodeTimeoutID = setTimeout(flushCleanQueue, DELAY_MS) + cleanNodeTimeoutID = setTimeout(flushBatch, DELAY_MS) } } -function flushCleanQueue() { +function flushBatch() { cleanNodeTimeoutID = null - const nodes = cleanNodeQueue.splice(0, MAX_CLEAN_AT_ONCE) + // If the option was flipped to a non-positive / non-finite value while the + // timer was pending, fall through to synchronous drain — otherwise + // splice(0, <=0|NaN) removes nothing and scheduleBatch re-arms forever. + const batchSize = Math.trunc(options.jsxCleanBatchSize) + if (!Number.isFinite(batchSize) || batchSize <= 0) { + flushAll() + return + } + const nodes = cleanNodeQueue.splice(0, batchSize) for (const node of nodes) { cleanNode(node) } - triggerCleanTimeout() + scheduleBatch() +} + +function flushAll() { + if (cleanNodeTimeoutID !== null) { + clearTimeout(cleanNodeTimeoutID) + cleanNodeTimeoutID = null + } + // Outer `while` is for re-enqueues triggered by cleanNode side effects. + while (cleanNodeQueue.length) { + for (const node of cleanNodeQueue.splice(0)) { + cleanNode(node) + } + } } diff --git a/tko.io/public/agents/guide.md b/tko.io/public/agents/guide.md index d5f363d9..3346dcbb 100644 --- a/tko.io/public/agents/guide.md +++ b/tko.io/public/agents/guide.md @@ -4,6 +4,8 @@ Test-backed behavior summaries live under `/agents/verified-behaviors/`. Treat t For preferred state/binding/DOM architecture in examples and prototypes, read `/agents/contract.md`. +For how to add or configure runtime options (`ko.options.*`), read `/agents/options.md`. + ## Setup ```html diff --git a/tko.io/public/agents/options.md b/tko.io/public/agents/options.md new file mode 100644 index 00000000..88d21d03 --- /dev/null +++ b/tko.io/public/agents/options.md @@ -0,0 +1,66 @@ +# `ko.options.*` — Configurable Runtime Options + +`ko.options` is TKO's runtime configuration object. It is a singleton `Options` +instance defined in `packages/utils/src/options.ts`. This page documents how +to add new options and which of the two available mechanisms to reach for. + +## Two mechanisms — when to use which + +### `defineOption` (default — almost always use this) + +For options that belong to a specific package (batch sizes, feature toggles, +plugin behavior), register via `defineOption` inside the owning package. + +```ts +// packages/utils.jsx/src/jsxClean.ts +import { defineOption, options } from '@tko/utils' + +// Extend the Options type so ko.options. is strongly typed. +declare module '@tko/utils' { + interface Options { + jsxCleanBatchSize: number + } +} + +// Register at module load, with an optional side-effect setter. +defineOption('jsxCleanBatchSize', { default: 1000 }) + +// Read wherever the option applies. +if (options.jsxCleanBatchSize === 0) { /* sync path */ } +``` + +Rules: + +- The `declare module '@tko/utils' { interface Options { ... } }` augmentation + lives in the same file (or at least the same package) that defines the option. +- `defineOption` registers at module-side-effect time; the option is available + as soon as the owning package is imported. +- An optional `set(value)` runs side effects at configuration time (not + retroactively on already-parsed bindings) — use for options that swap + implementations or rebuild internal state. + +**Canonical example with a side-effect setter:** +`packages/utils.parser/src/operators.ts` → `strictEquality` swaps the `==` +and `!=` operator implementations when the option flips. + +### Core `Options` class field (rare — only for `@tko/utils` intrinsics) + +Add a field directly to the `Options` class in `packages/utils/src/options.ts` +**only** when the option is intrinsic to `@tko/utils` itself — something that +`@tko/utils` uses or enforces without knowing about any downstream package. + +Current core fields include `templateSizeLimit`, `deferUpdates`, +`useOnlyNativeEvents`, `onError`, `sanitizeHtmlTemplate`. + +Never add a core-class field for an option that belongs to another package. +Doing so forces `@tko/utils` to carry concepts from downstream packages and +creates a wrong-way dependency. + +## Quick test + +> Is the option intrinsic to `@tko/utils`? + +- Yes → core class field. +- No → `defineOption` in the owning package. + +In practice, new options are almost always the second case. diff --git a/tko.io/public/llms.txt b/tko.io/public/llms.txt index 49ffe591..b6d1ce88 100644 --- a/tko.io/public/llms.txt +++ b/tko.io/public/llms.txt @@ -17,6 +17,7 @@ - /agents/glossary.md — domain terms, concepts, packages - /agents/testing.md — how to run and verify TKO code - /agents/contract.md — state/binding/DOM architecture +- /agents/options.md — ko.options.* — when to use defineOption vs core Options fields - /agents/verified-behaviors/index.md — test-backed behavior contracts - /agents/sample-tsx.html — minimal browser TSX scaffold - /examples/ — interactive self-contained HTML examples