From 2683d9370467ee868590a87aa8d04f2262165444 Mon Sep 17 00:00:00 2001 From: Jack Date: Mon, 18 May 2026 15:08:53 -0400 Subject: [PATCH 01/19] Feat: Add signal safety as single commit --- .../advanced/reactive-notify/index.js | 2 +- .../variables/reactive-clone/index.js | 39 +- docs/src/pages/docs/api/reactivity/signal.mdx | 57 +- .../docs/guides/reactivity/signal-options.mdx | 71 +- .../pages/docs/guides/reactivity/signals.mdx | 38 + .../bench/tachometer/bench-krausest.js | 2 +- .../bench/tachometer/bench-template.js | 2 +- packages/reactivity/src/signal.js | 301 +++--- .../reactivity/test/unit/internals.test.js | 11 +- .../reactivity/test/unit/reaction.test.js | 4 +- .../test/unit/signal-helpers.test.js | 890 ++++++++++++++++++ packages/reactivity/test/unit/signal.test.js | 800 +--------------- packages/renderer/src/engines/lit/renderer.js | 2 +- .../src/engines/native/reactive-context.js | 3 +- .../test/browser/subtree-spurious.test.js | 8 +- packages/templating/src/template.js | 4 +- packages/templating/test/data-context.test.js | 2 +- .../test/subtemplate-settings.test.js | 2 +- packages/utils/src/functions.js | 5 + tools/benchmark/src/main.js | 4 +- 20 files changed, 1229 insertions(+), 1018 deletions(-) create mode 100644 packages/reactivity/test/unit/signal-helpers.test.js diff --git a/docs/src/examples/reactivity/advanced/reactive-notify/index.js b/docs/src/examples/reactivity/advanced/reactive-notify/index.js index 077e5785a..416282fdf 100644 --- a/docs/src/examples/reactivity/advanced/reactive-notify/index.js +++ b/docs/src/examples/reactivity/advanced/reactive-notify/index.js @@ -1,6 +1,6 @@ import { Reaction, Signal } from '@semantic-ui/reactivity'; -const data = new Signal({ count: 0 }, { allowClone: false }); +const data = new Signal({ count: 0 }, { safety: 'reference' }); Reaction.create(() => { console.log('Count:', data.get().count); diff --git a/docs/src/examples/reactivity/variables/reactive-clone/index.js b/docs/src/examples/reactivity/variables/reactive-clone/index.js index 7abbacb35..d6c9b539c 100644 --- a/docs/src/examples/reactivity/variables/reactive-clone/index.js +++ b/docs/src/examples/reactivity/variables/reactive-clone/index.js @@ -1,29 +1,34 @@ /* - By default accidental mutations are protected because set/get are cloned - using allowClone: false will remove this protection but avoid cloning overhead + Signals default to safety: 'freeze' — the stored value is deep-frozen, + so accidental in-place mutation throws at the call site. + Pass safety: 'reference' to opt a signal out of freezing (e.g. when it + holds third-party objects that mutate their own state). */ import { Reaction, Signal } from '@semantic-ui/reactivity'; -const safeItems = new Signal(['apple', 'banana']); -const unsafeItems = new Signal(['apple', 'banana'], { allowClone: false }); +const safe = new Signal(['apple', 'banana']); +const unsafe = new Signal(['apple', 'banana'], { safety: 'reference' }); Reaction.create(() => { - console.log('Safe items:', safeItems.get()); + console.log('Safe items:', safe.get()); }); Reaction.create(() => { - console.log('Unsafe items:', unsafeItems.get()); + console.log('Unsafe items:', unsafe.get()); }); -console.log('--- Accidental mutation ---'); - -// Get references and try to mutate them -const safeRef = safeItems.get(); -const unsafeRef = unsafeItems.get(); +// Correct way to add an item under either mode: use the mutation helper +safe.push('orange'); +unsafe.push('orange'); +Reaction.flush(); -// somewhere else in code accidentally mutate the copy -safeRef.push('cherry'); -unsafeRef.push('cherry'); +// Direct mutation on the reference: throws under freeze, silent no-op under reference +try { + safe.get().push('cherry'); +} +catch (e) { + console.log('Caught:', e.message); +} -safeItems.push('orange'); -unsafeItems.push('orange'); -Reaction.flush(); +// Under reference, this mutates the stored array in place but does NOT notify subscribers +unsafe.get().push('cherry'); +console.log('Unsafe after silent mutation:', unsafe.peek()); diff --git a/docs/src/pages/docs/api/reactivity/signal.mdx b/docs/src/pages/docs/api/reactivity/signal.mdx index 254ea1eb4..e7f84a4bc 100755 --- a/docs/src/pages/docs/api/reactivity/signal.mdx +++ b/docs/src/pages/docs/api/reactivity/signal.mdx @@ -30,9 +30,9 @@ new Signal(initialValue, options); | Name | Type | Default | Description | |------|------|---------|-------------| +| safety | `'freeze'` \| `'reference'` \| `'none'` | `'freeze'` | Value-protection preset. See [Signal Options](/docs/guides/reactivity/signal-options) for details. | | equalityFunction | Function | isEqual | Custom function to determine if the value has changed | -| allowClone | boolean | true | Whether to clone the value when getting/setting | -| cloneFunction | Function | clone | Custom function to clone the value | +| cloneFunction | Function | clone | Custom function used by `signal.clone()` | ### Usage @@ -54,28 +54,18 @@ const person = new Signal({ name: 'John', age: 30 }, { }); ``` -#### Disabling Cloning for Custom Classes +#### Holding Borrowed Data -To avoid mutating the source object naively, by default values are cloned when set. However some objects cannot be naively cloned like custom classes. +The default `safety: 'freeze'` deep-freezes object and array values on `set` to catch accidental in-place mutation. For signals holding objects you don't own — anything returned from a library, fetched from an API, or passed through a callback — opt out with `safety: 'reference'` so the library's own reference isn't poisoned. ```javascript import { Signal } from '@semantic-ui/reactivity'; -class CustomClass { - constructor(value) { - this.value = value; - } -} - -const customInstance = new Signal(new CustomClass(42), { - allowClone: false, - equalityFunction: (a, b) => a.value === b.value -}); - -// The CustomClass instance won't be cloned when accessed -console.log(customInstance.get().value); // Output: 42 +const searchResults = new Signal(pagefindResults, { safety: 'reference' }); ``` +See the [Signals and Foreign References](/docs/guides/reactivity/signals#signals-and-foreign-references) section of the signals guide for the full heuristic. + ## `Get` Returns the current value of the reactive variable. @@ -212,7 +202,7 @@ someValue.notify(); ```javascript import { Reaction, Signal } from '@semantic-ui/reactivity'; -const canvas = new Signal(document.createElement('canvas'), { allowClone: false }); +const canvas = new Signal(document.createElement('canvas'), { safety: 'reference' }); Reaction.create(() => { const el = canvas.get(); @@ -298,7 +288,7 @@ console.log(counter.get()); // Output: undefined ## `Mutate` -Safely mutates the current value using a mutation function. This method ensures reactivity is triggered even when `allowClone` is false or when using custom equality functions. +Safely mutates the current value using a mutation function. Under `safety: 'freeze'` the mutation function must return a new value (in-place mutation throws). Under `safety: 'reference'` or `'none'` the function may mutate in place and reactivity still fires. ### Syntax ```javascript @@ -317,24 +307,28 @@ The return value of the mutation function, if any. ### Usage -#### In-place Mutation +#### Returning a New Value (works under all safety modes) ```javascript import { Signal } from '@semantic-ui/reactivity'; const items = new Signal(['apple', 'banana']); -items.mutate(arr => { - arr.push('orange'); // Mutate in place -}); +items.mutate(arr => [...arr, 'orange']); console.log(items.get()); // ['apple', 'banana', 'orange'] + +const count = new Signal(5); +count.mutate(val => val * 2); +console.log(count.get()); // 10 ``` -#### Returning a New Value +#### In-place Mutation (only under safety: 'reference' or 'none') ```javascript import { Signal } from '@semantic-ui/reactivity'; -const count = new Signal(5); -count.mutate(val => val * 2); // Return new value -console.log(count.get()); // 10 +const items = new Signal(['apple', 'banana'], { safety: 'reference' }); +items.mutate(arr => { + arr.push('orange'); // Mutate in place — allowed because the value isn't frozen +}); +console.log(items.get()); // ['apple', 'banana', 'orange'] ``` #### With Custom Classes @@ -350,12 +344,13 @@ class Counter { } } -const counter = new Signal(new Counter(0), { - allowClone: false, - equalityFunction: (a, b) => a.value === b.value +// Class instances aren't frozen by default (deepFreeze skips them), +// but set safety: 'reference' explicitly when you plan to mutate in place +const counter = new Signal(new Counter(0), { + safety: 'reference', + equalityFunction: (a, b) => a.value === b.value, }); -// Safe mutation that triggers reactivity counter.mutate(c => c.increment()); console.log(counter.get().value); // 1 ``` diff --git a/docs/src/pages/docs/guides/reactivity/signal-options.mdx b/docs/src/pages/docs/guides/reactivity/signal-options.mdx index f36619254..47608ded0 100644 --- a/docs/src/pages/docs/guides/reactivity/signal-options.mdx +++ b/docs/src/pages/docs/guides/reactivity/signal-options.mdx @@ -44,60 +44,59 @@ const customEquality = (a, b) => { const customVar = new Signal(initialValue, { equalityFunction: customEquality }); ``` -## Value Cloning +## Safety Presets -By default, Signals automatically clone object and array values during [`get`](/docs/api/reactivity/signal#get) and [`set`](/docs/api/reactivity/signal#set) operations. +Signals have three safety presets controlling how the stored value is protected against accidental mutation. Set the preset via the `safety` option. -A very common issue when using naive Signals implementations is that it can be very easy to accidentally update an underlying signal value when modifying its value without using `set()` or `value`. +| Preset | On `set` | On `.get().x = y` | Dedupe | Use case | +|---|---|---|---|---| +| `freeze` (default) | deep-freeze plain objects and arrays | throws `TypeError` | `isEqual` | state your code owns end-to-end | +| `reference` | store raw | silent (no reactivity) | `isEqual` | third-party objects, perf-critical paths | +| `none` | store raw | silent (no reactivity) | never | event-stream semantics — every `set` notifies | -```javascript -const countObj = new Signal({ count: 0 }); +### `safety: 'freeze'` — the default -// by default this will not update the current count unless set() is called -const newObj = countObj.get(); -newObj.count = 1; -``` +The default deep-freezes object and array values when you call `set()`. Accidental in-place mutation throws at the call site instead of silently dropping the update. -Cloning prevents accidental mutation of the Signal's internal state and ensures reliable [equality checks](#equality-comparison). +```javascript +const count = new Signal({ n: 0 }); -Disabling cloning (using the [`allowClone`](/docs/api/reactivity/signal#options) option) will reduce the overhead of `set` and `get` calls but will potentially cause unexpected behaviors when modifying arrays and objects. -```javascript title="Accidental Mutations" -const countObj = new Signal({ count: 0 }, { allowClone: false }); +count.get().n = 1; // TypeError — "Signal value is frozen — cannot set property `n`" -// this will not trigger reactivity, but the value will change in the next flush -// this is because the underlying signal was accidentally mutated -const newObj = countObj.get(); -newObj.count = 1; +// Correct ways to update: +count.set({ n: 1 }); // replace the whole value +count.mutate(prev => ({ n: prev.n + 1 })); // return a new value +count.setProperty('n', 1); // mutation helper — rebuilds immutably under freeze ``` -### Custom Cloning Function +Deep-freeze only walks arrays and plain objects. `Date`, `Map`, `Set`, `RegExp`, DOM nodes, and class instances are stored by reference — their own mutation semantics are preserved. -Similar to the equality check, the cloning function itself can be customized by providing a [`cloneFunction`](/docs/api/reactivity/signal#options) in the constructor options. +### `safety: 'reference'` — opt-out for borrowed data -```javascript -// Simple JSON clone -const customClone = (value) => { - return JSON.parse(JSON.stringify(value)); -}; +Use `reference` when the signal holds objects you didn't construct yourself. Freezing them would poison the lender's internal references; see [Signals and Foreign References](/docs/guides/reactivity/signals#signals-and-foreign-references) for the full heuristic. -// Signal using custom clone function -const customCloneSignal = new Signal({ data: 1 }, { cloneFunction: customClone }); +```javascript +const searchResults = new Signal([], { safety: 'reference' }); ``` -### Uncloneable Content +Direct mutation on `.get()` values fails silently under `reference` — the helpers (`push`, `splice`, `setProperty`) remain the safe update path. -Some values do not have reliable ways to clone. For instance, there is no universal way to clone a custom class. +### `safety: 'none'` — event-stream semantics -**Class instances are not cloned** by default, regardless of the `allowClone` setting. Signals always store and return direct references to class instances. +Use `none` when every `set` should notify subscribers, even if the value is deeply equal to the previous one. Suitable for notification channels where the payload's shape repeats. ```javascript -class MyData { - constructor(value) { this.value = value; } -} +const pulse = new Signal(null, { safety: 'none' }); + +pulse.set({ type: 'heartbeat' }); +pulse.set({ type: 'heartbeat' }); // still notifies, even though isEqual would say equal +``` -const instanceSignal = new Signal(new MyData(10)); +## Custom Clone Function -const instance1 = instanceSignal.get(); -const instance2 = instanceSignal.get(); -console.log(instance1 === instance2); +The default `cloneFunction` is used by `signal.clone()` to produce a deep-mutable copy on demand. Override it if you need non-default clone semantics. + +```javascript +const jsonClone = (value) => JSON.parse(JSON.stringify(value)); +const mySignal = new Signal({ data: 1 }, { cloneFunction: jsonClone }); ``` diff --git a/docs/src/pages/docs/guides/reactivity/signals.mdx b/docs/src/pages/docs/guides/reactivity/signals.mdx index 897da1f79..f76a4caf5 100644 --- a/docs/src/pages/docs/guides/reactivity/signals.mdx +++ b/docs/src/pages/docs/guides/reactivity/signals.mdx @@ -67,6 +67,44 @@ counter.increment(); // Equivalent to counter.set(counter.peek() + 1) console.log(counter.get()); // Output: 2 ``` +## Signals and Foreign References + +By default, Signals deep-freeze object and array values on `set()`. This catches the most common reactivity bug — mutating a value in place without notifying subscribers — by throwing a `TypeError` at the mutation site instead of silently dropping the update. + +Deep-freezing has one important caveat: if the object you store is *also held internally by a library*, freezing it will break that library the next time it mutates its own reference. + +> **When you need `{ safety: 'reference' }`**: if you're storing an object in a signal that you did not construct yourself — anything returned from a library, fetched from an API, or passed through a callback — default to `safety: 'reference'`. Freeze is the right default for state your own code owns end-to-end. For borrowed data, `reference` avoids poisoning the lender's internal references. + +### Worked Example: Search Index + +Pagefind returns result objects and continues to use them internally — subsequent `.data()` calls on each result mutate pagefind's own cached state. Storing the results under the default freeze mode freezes pagefind's internal objects and later crashes its loader: + +```javascript +const defaultState = { + // ❌ default freeze — pagefind's internal mutation of the stored objects will throw + rawResults: [], +}; +``` + +Opt this specific signal out of freeze: + +```javascript +const defaultState = { + // ✓ signal holds third-party-owned data; don't freeze + rawResults: { value: [], options: { safety: 'reference' } }, +}; +``` + +The rest of your component state keeps the default freeze protection — only the signal carrying borrowed data opts out. + +### Ad-hoc Construction + +For signals created outside a `defaultState` declaration, pass the preset as the second argument: + +```javascript +const results = new Signal(pagefindData, { safety: 'reference' }); +``` + ## Creating Derived Signals Signals can be transformed and combined to create new reactive values: diff --git a/packages/component/bench/tachometer/bench-krausest.js b/packages/component/bench/tachometer/bench-krausest.js index 322306095..9b3fa2629 100644 --- a/packages/component/bench/tachometer/bench-krausest.js +++ b/packages/component/bench/tachometer/bench-krausest.js @@ -110,7 +110,7 @@ function buildData(count) { Component Definition *******************************/ -const signalOptions = { allowClone: false, safety: 'reference' }; +const signalOptions = { safety: 'reference' }; defineComponent({ tagName: 'bench-app', diff --git a/packages/component/bench/tachometer/bench-template.js b/packages/component/bench/tachometer/bench-template.js index c955ecb70..29e65cf47 100644 --- a/packages/component/bench/tachometer/bench-template.js +++ b/packages/component/bench/tachometer/bench-template.js @@ -109,7 +109,7 @@ defineComponent({ defaultState: { items: { value: Array.from({ length: 500 }, (_, i) => ({ id: i, label: `item-${i}` })), - options: { allowClone: false, safety: 'reference' }, + options: { safety: 'reference' }, }, }, }); diff --git a/packages/reactivity/src/signal.js b/packages/reactivity/src/signal.js index 71335c5c7..34845692c 100755 --- a/packages/reactivity/src/signal.js +++ b/packages/reactivity/src/signal.js @@ -6,12 +6,14 @@ import { isEqual, isNumber, isObject, + returnsFalse, unique, wrapFunction, } from '@semantic-ui/utils'; -import { Dependency } from './dependency.js'; import { captureStack, isStackCapture, isTracing, setStackCapture, setTracing } from './helpers.js'; + +import { Dependency } from './dependency.js'; import { Reaction } from './reaction.js'; const IS_SIGNAL = Symbol.for('semantic-ui/Signal'); @@ -24,110 +26,58 @@ export class Signal { return !!instance?.[IS_SIGNAL]; } - constructor(initialValue, { context, equalityFunction, allowClone = true, cloneFunction } = {}) { - // pass in some metadata for debugging + // default clone and equal pulled from utils + static equalityFunction = isEqual; + static cloneFunction = clone; + static safety = 'clone'; + + constructor(initialValue, { + safety = Signal.safety, + cloneFunction = Signal.cloneFunction, + equalityFunction = (safety === 'none') ? returnsFalse : Signal.equalityFunction, + context, + } = {}) { + // create dependency this.dependency = new Dependency({ firstRun: true, value: initialValue, }); - // allow user to opt out of value cloning - this.allowClone = allowClone; + // handle custom helpers if user specifies + this.cloneFunction = cloneFunction; + this.equalityFunction = equalityFunction; - // allow custom equality function - this.equalityFunction = (equalityFunction) - ? wrapFunction(equalityFunction) - : Signal.equalityFunction; + this.safety = safety; + this.currentValue = this.protect(initialValue); - // allow custom clone function - this.clone = (cloneFunction) - ? wrapFunction(cloneFunction) - : Signal.cloneFunction; - - this.currentValue = this.maybeClone(initialValue); - - // allow debugging context to be set + // pass through debugging context this.setContext(context); } - // set debugging context for signal removing any present context - setContext(additionalContext = {}) { - if (!isTracing()) { - return; - } - const defaultContext = { - value: this.currentValue, - }; - this.context = { - ...defaultContext, - ...additionalContext, - }; - } - - // add context to signal - addContext(additionalContext = {}) { - if (!isTracing()) { - return; - } - if (!this.context) { - this.context = {}; + protect(value) { + if (value === null || typeof value !== 'object') { + return value; } - for (const key in additionalContext) { - this.context[key] = additionalContext[key]; + if (this.safety === 'clone') { + return this.clone(value); } + return value; } - // Stack trace capture is gated separately because Error.captureStackTrace - // costs ~10-100× a context spread, paid per Signal.notify in tracing-on - // dev. Default off; opt in via setStackCapture(true). - setTrace() { - captureStack(this, this.setTrace); - } - - static equalityFunction = isEqual; - static cloneFunction = clone; - static setTracing = setTracing; - static isTracing = isTracing; - static setStackCapture = setStackCapture; - static isStackCapture = isStackCapture; - get value() { // Record this Signal as a dependency if inside a Reaction computation this.depend(); - const value = this.currentValue; - - // otherwise previous value would be modified if the returned value is mutated negating the equality - return (value !== null && typeof value == 'object') - ? this.maybeClone(value) - : value; - } - - canCloneValue(value) { - return (this.allowClone === true && !isClassInstance(value)); - } - - maybeClone(value) { - if (!this.canCloneValue(value)) { - return value; - } - if (isArray(value)) { - return value.map(value => this.maybeClone(value)); - } - return this.clone(value); + return this.protect(this.currentValue); } set value(newValue) { if (!this.equalityFunction(this.currentValue, newValue)) { - this.currentValue = this.maybeClone(newValue); + this.currentValue = this.protect(newValue); this.notify(); } } - get({ clone = true } = {}) { - if (!clone) { - this.depend(); - return this.currentValue; - } + get() { return this.value; } @@ -136,26 +86,66 @@ export class Signal { this.value = newValue; } + notify() { + this.setContext(); + this.setTrace(); + this.dependency.changed(this.context); + } + + peek() { + return this.protect(this.currentValue); + } + + raw() { + return this.currentValue; + } + + clone(value = this.currentValue) { + if (isClassInstance(value)) { + return value; + } + if (isArray(value)) { + return value.map(arrValue => this.clone(arrValue)); + } + return this.cloneFunction(value); + } + subscribe(callback) { return Reaction.create((comp) => { callback(this.value, comp); }); } - // derive a new signal from this signal's value + /* Dependencies */ + hasDependents() { + return this.dependency.subscribers.size > 0; + } + + depend() { + this.dependency.depend(); + } + + /******************************* + Child Signals + *******************************/ + + // single signal having a derivation derive(computeFn, options = {}) { const derivedSignal = new Signal(undefined, options); - // weak so the reaction's closure doesn't pin derived through source.dep.subscribers + + // weak so the reaction's closure doesn't pin derived + // through source.dep.subscribers const derivedRef = new WeakRef(derivedSignal); + const source = this; const reaction = Reaction.create(() => { - const d = derivedRef.deref(); - if (!d) { + const currentRef = derivedRef.deref(); + if (!currentRef) { reaction.stop(); return; } - d.set(computeFn(source.get())); + currentRef.set(computeFn(source.get())); }); if (Reaction.current) { @@ -165,18 +155,18 @@ export class Signal { return derivedSignal; } - // static method for computing from multiple signals + // multiple signals computing a signal static computed(computeFn, options = {}) { const computedSignal = new Signal(undefined, options); const computedRef = new WeakRef(computedSignal); const reaction = Reaction.create(() => { - const c = computedRef.deref(); - if (!c) { + const ref = computedRef.deref(); + if (!ref) { reaction.stop(); return; } - c.set(computeFn()); + ref.set(computeFn()); }); if (Reaction.current) { @@ -186,29 +176,9 @@ export class Signal { return computedSignal; } - depend() { - this.dependency.depend(); - } - - notify() { - // Each gate handles itself — setContext on isTracing, setTrace on - // isStackCapture. Hot path: both early-return when their flag is off. - this.setContext(); - this.setTrace(); - this.dependency.changed(this.context); - } - - hasDependents() { - return this.dependency.subscribers.size > 0; - } - - peek() { - return this.maybeClone(this.currentValue); - } - - clear() { - return this.set(undefined); - } + /******************************* + Mutation Helpers + *******************************/ // mutate the current value by a mutation function mutate(mutationFn) { @@ -234,6 +204,11 @@ export class Signal { } } + // clears current value + clear() { + return this.set(undefined); + } + // array helpers — these always change the value, skip clone+compare push(...args) { this.currentValue.push(...args); @@ -248,11 +223,21 @@ export class Signal { this.notify(); } map(mapFunction) { - this.currentValue = Array.prototype.map.call(this.currentValue, mapFunction); + const arr = this.currentValue; + for (let i = 0; i < arr.length; i++) { + arr[i] = mapFunction(arr[i], i, arr); + } this.notify(); } filter(filterFunction) { - this.currentValue = Array.prototype.filter.call(this.currentValue, filterFunction); + const arr = this.currentValue; + let writeIndex = 0; + for (let i = 0; i < arr.length; i++) { + if (filterFunction(arr[i], i, arr)) { + arr[writeIndex++] = arr[i]; + } + } + arr.length = writeIndex; this.notify(); } @@ -260,6 +245,9 @@ export class Signal { return this.get()[index]; } setIndex(index, value) { + if (this.currentValue[index] === value) { + return; + } this.currentValue[index] = value; this.notify(); } @@ -268,7 +256,6 @@ export class Signal { this.notify(); } - // sets setArrayProperty(indexOrProperty, property, value) { let index; if (isNumber(indexOrProperty)) { @@ -282,13 +269,25 @@ export class Signal { if (index === -1) { return; } - const newValue = this.peek().map((object, currentIndex) => { - if (index == 'all' || currentIndex == index) { - object[property] = value; + const arr = this.currentValue; + let changed = false; + if (index === 'all') { + for (let i = 0; i < arr.length; i++) { + if (arr[i][property] !== value) { + arr[i][property] = value; + changed = true; + } } - return object; - }); - this.set(newValue); + } + else { + if (arr[index][property] !== value) { + arr[index][property] = value; + changed = true; + } + } + if (changed) { + this.notify(); + } } toggle() { @@ -341,8 +340,10 @@ export class Signal { } getItemIndex(id) { const arr = this.currentValue; - for (let i = 0; i < arr.length; i++) { - if (this.hasID(arr[i], id)) { return i; } + for (let index = 0; index < arr.length; index++) { + if (this.hasID(arr[index], id)) { + return index; + } } return -1; } @@ -355,9 +356,11 @@ export class Signal { else { value = property; property = idOrProperty; - const obj = this.peek(); - obj[property] = value; - this.set(obj); + if (this.currentValue[property] === value) { + return; + } + this.currentValue[property] = value; + this.notify(); } } replaceItem(id, item) { @@ -366,4 +369,50 @@ export class Signal { removeItem(id) { return this.removeIndex(this.getItemIndex(id)); } + + /******************************* + Tracing Utils + *******************************/ + + static setTracing = setTracing; + static isTracing = isTracing; + static setStackCapture = setStackCapture; + static isStackCapture = isStackCapture; + + // context lets you pass through metadata with a signal + // to determine reaction source + setContext(additionalContext) { + if (!isTracing()) { + return; + } + const defaultContext = { + value: this.currentValue, + }; + if (!additionalContext) { + this.addContext(defaultContext); + } + else { + this.context = { + ...defaultContext, + ...additionalContext, + }; + } + } + addContext(additionalContext = {}) { + if (!isTracing()) { + return; + } + if (!this.context) { + this.context = {}; + } + for (const key in additionalContext) { + this.context[key] = additionalContext[key]; + } + } + + // capturing stack pays a 10-100× perf cost + // opt in only via setStackCapture(true). + setTrace() { + captureStack(this, this.setTrace); + } } diff --git a/packages/reactivity/test/unit/internals.test.js b/packages/reactivity/test/unit/internals.test.js index c43a5fee7..08b0b19b8 100644 --- a/packages/reactivity/test/unit/internals.test.js +++ b/packages/reactivity/test/unit/internals.test.js @@ -765,19 +765,24 @@ describe('mid-reaction signal updates', () => { *******************************/ describe('Signal — read paths via internals', () => { - it('get({ clone: false }) creates a dependency but returns the live reference', () => { + it('raw() returns the live reference without creating a dependency', () => { const obj = { a: 1 }; const s = new Signal(obj); let lastSeen; + let runs = 0; Reaction.create(() => { - lastSeen = s.get({ clone: false }); + runs++; + lastSeen = s.raw(); }); + expect(runs).toBe(1); expect(lastSeen).toBe(s.currentValue); s.set({ a: 2 }); Reaction.flush(); - expect(lastSeen.a).toBe(2); + // raw() did not subscribe — reaction does not re-run + expect(runs).toBe(1); + expect(lastSeen.a).toBe(1); }); it('peek returns a clone (defensive read) but no dependency', () => { diff --git a/packages/reactivity/test/unit/reaction.test.js b/packages/reactivity/test/unit/reaction.test.js index 71eae5987..b627ca491 100644 --- a/packages/reactivity/test/unit/reaction.test.js +++ b/packages/reactivity/test/unit/reaction.test.js @@ -258,8 +258,8 @@ describe('Reaction', () => { expect(callback).toHaveBeenCalledTimes(3); }); - it('should correctly manipulate array with helpers when allowClone is false', () => { - const items = new Signal([0, 1, 2], { allowClone: false }); + it('should correctly manipulate array with helpers when cloned', () => { + const items = new Signal([0, 1, 2], { safety: 'clone' }); const callback = vi.fn(); Reaction.create(() => { diff --git a/packages/reactivity/test/unit/signal-helpers.test.js b/packages/reactivity/test/unit/signal-helpers.test.js new file mode 100644 index 000000000..1a5eb8d2d --- /dev/null +++ b/packages/reactivity/test/unit/signal-helpers.test.js @@ -0,0 +1,890 @@ +import { Reaction, Signal } from '@semantic-ui/reactivity'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +// Suite contract: +// Every mutation helper must (1) apply its mutation, (2) fire reactivity, and +// (3) use the in-place / reference path internally — even under safety: 'clone'. +// The third property is checked by capturing the live storage ref via raw() +// before and after the helper call. If a helper replaces storage instead of +// mutating it, the post-call raw() will be a different ref and the test fails. +// +// We deliberately do NOT read signal.js while authoring this — these are +// blackbox expectations a downstream user has reading the public d.ts. + +const SAFETY_MODES = ['clone', 'reference']; + +SAFETY_MODES.forEach((safety) => { + describe(`Signal mutation helpers — safety: '${safety}'`, () => { + /******************************* + Test helpers + *******************************/ + + // Track how many times a reaction re-runs after the initial registration. + // Reaction.create runs the fn once synchronously to capture deps; we + // discount that first run so .count reflects only post-helper wakeups. + const subscribe = (signal) => { + let runs = 0; + const reaction = Reaction.create(() => { + signal.get(); + runs++; + }); + return { + get count() { + return runs - 1; + }, + stop: () => reaction.stop(), + }; + }; + + /******************************* + Array container helpers + *******************************/ + + describe('push', () => { + it('appends items to the array', () => { + const sig = new Signal([1, 2, 3], { safety }); + sig.push(4); + expect(sig.raw()).toEqual([1, 2, 3, 4]); + }); + + it('fires reactivity', () => { + const sig = new Signal([1, 2, 3], { safety }); + const sub = subscribe(sig); + sig.push(4); + Reaction.flush(); + expect(sub.count).toBe(1); + sub.stop(); + }); + + it('preserves stored array identity (in-place mutation)', () => { + const sig = new Signal([1, 2, 3], { safety }); + const before = sig.raw(); + sig.push(4); + expect(sig.raw()).toBe(before); + }); + }); + + describe('unshift', () => { + it('prepends items to the array', () => { + const sig = new Signal([1, 2, 3], { safety }); + sig.unshift(0); + expect(sig.raw()).toEqual([0, 1, 2, 3]); + }); + + it('fires reactivity', () => { + const sig = new Signal([1, 2, 3], { safety }); + const sub = subscribe(sig); + sig.unshift(0); + Reaction.flush(); + expect(sub.count).toBe(1); + sub.stop(); + }); + + it('preserves stored array identity (in-place mutation)', () => { + const sig = new Signal([1, 2, 3], { safety }); + const before = sig.raw(); + sig.unshift(0); + expect(sig.raw()).toBe(before); + }); + }); + + describe('splice', () => { + it('removes and inserts at the given index', () => { + const sig = new Signal([1, 2, 3, 4], { safety }); + sig.splice(1, 2, 20, 30); + expect(sig.raw()).toEqual([1, 20, 30, 4]); + }); + + it('inserts without removing when deleteCount is 0', () => { + const sig = new Signal(['a', 'b', 'c', 'd'], { safety }); + sig.splice(2, 0, 'x', 'y'); + expect(sig.raw()).toEqual(['a', 'b', 'x', 'y', 'c', 'd']); + }); + + it('removing the entire array yields an empty array', () => { + const sig = new Signal(['a', 'b', 'c'], { safety }); + sig.splice(0, 3); + expect(sig.raw()).toEqual([]); + }); + + it('fires reactivity', () => { + const sig = new Signal([1, 2, 3, 4], { safety }); + const sub = subscribe(sig); + sig.splice(1, 1); + Reaction.flush(); + expect(sub.count).toBe(1); + sub.stop(); + }); + + it('preserves stored array identity (in-place mutation)', () => { + const sig = new Signal([1, 2, 3, 4], { safety }); + const before = sig.raw(); + sig.splice(1, 1); + expect(sig.raw()).toBe(before); + }); + }); + + describe('map', () => { + it('transforms every element', () => { + const sig = new Signal([1, 2, 3], { safety }); + sig.map(x => x * 10); + expect(sig.raw()).toEqual([10, 20, 30]); + }); + + it('handles complex transformations producing new object shapes', () => { + const sig = new Signal( + [{ id: 1, value: 10 }, { id: 2, value: 20 }], + { safety }, + ); + sig.map(item => ({ ...item, doubled: item.value * 2 })); + expect(sig.raw()).toEqual([ + { id: 1, value: 10, doubled: 20 }, + { id: 2, value: 20, doubled: 40 }, + ]); + }); + + it('fires reactivity', () => { + const sig = new Signal([1, 2, 3], { safety }); + const sub = subscribe(sig); + sig.map(x => x + 1); + Reaction.flush(); + expect(sub.count).toBe(1); + sub.stop(); + }); + + it('preserves stored array identity (in-place mutation)', () => { + const sig = new Signal([1, 2, 3], { safety }); + const before = sig.raw(); + sig.map(x => x + 1); + expect(sig.raw()).toBe(before); + }); + }); + + describe('filter', () => { + it('keeps only matching elements', () => { + const sig = new Signal([1, 2, 3, 4], { safety }); + sig.filter(x => x % 2 === 0); + expect(sig.raw()).toEqual([2, 4]); + }); + + it('filtering to an empty result yields an empty array, not undefined', () => { + const sig = new Signal([1, 2, 3], { safety }); + sig.filter(() => false); + expect(sig.raw()).toEqual([]); + }); + + it('handles complex objects via predicate', () => { + const sig = new Signal( + [{ id: 1, active: true }, { id: 2, active: false }, { id: 3, active: true }], + { safety }, + ); + sig.filter(item => item.active); + expect(sig.raw()).toEqual([ + { id: 1, active: true }, + { id: 3, active: true }, + ]); + }); + + it('fires reactivity', () => { + const sig = new Signal([1, 2, 3, 4], { safety }); + const sub = subscribe(sig); + sig.filter(x => x % 2 === 0); + Reaction.flush(); + expect(sub.count).toBe(1); + sub.stop(); + }); + + it('preserves stored array identity (in-place mutation)', () => { + const sig = new Signal([1, 2, 3, 4], { safety }); + const before = sig.raw(); + sig.filter(x => x > 1); + expect(sig.raw()).toBe(before); + }); + }); + + /******************************* + Index-addressed helpers + *******************************/ + + describe('getIndex', () => { + it('reads the element at the given index', () => { + const sig = new Signal(['red', 'green', 'blue'], { safety }); + expect(sig.getIndex(0)).toBe('red'); + expect(sig.getIndex(2)).toBe('blue'); + }); + + it('creates a dependency — setIndex writes re-fire the reaction', () => { + const sig = new Signal(['red', 'green', 'blue'], { safety }); + const fn = vi.fn(); + Reaction.create(() => fn(sig.getIndex(0))); + expect(fn).toHaveBeenCalledTimes(1); + expect(fn).toHaveBeenLastCalledWith('red'); + + sig.setIndex(0, 'purple'); + Reaction.flush(); + expect(fn).toHaveBeenCalledTimes(2); + expect(fn).toHaveBeenLastCalledWith('purple'); + }); + }); + + describe('setIndex', () => { + it('replaces the element at the given index', () => { + const sig = new Signal([1, 2, 3], { safety }); + sig.setIndex(1, 20); + expect(sig.raw()).toEqual([1, 20, 3]); + }); + + it('fires reactivity', () => { + const sig = new Signal([1, 2, 3], { safety }); + const sub = subscribe(sig); + sig.setIndex(1, 20); + Reaction.flush(); + expect(sub.count).toBe(1); + sub.stop(); + }); + + it('preserves stored array identity (in-place mutation)', () => { + const sig = new Signal([1, 2, 3], { safety }); + const before = sig.raw(); + sig.setIndex(1, 20); + expect(sig.raw()).toBe(before); + }); + + it('does not fire reactivity when the value is already what was passed', () => { + const sig = new Signal([1, 2, 3], { safety }); + const sub = subscribe(sig); + sig.setIndex(1, 2); + Reaction.flush(); + expect(sub.count).toBe(0); + sub.stop(); + }); + }); + + describe('removeIndex', () => { + it('removes the element at the given index', () => { + const sig = new Signal([1, 2, 3], { safety }); + sig.removeIndex(1); + expect(sig.raw()).toEqual([1, 3]); + }); + + it('fires reactivity', () => { + const sig = new Signal([1, 2, 3], { safety }); + const sub = subscribe(sig); + sig.removeIndex(1); + Reaction.flush(); + expect(sub.count).toBe(1); + sub.stop(); + }); + + it('preserves stored array identity (in-place mutation)', () => { + const sig = new Signal([1, 2, 3], { safety }); + const before = sig.raw(); + sig.removeIndex(1); + expect(sig.raw()).toBe(before); + }); + }); + + /******************************* + Collection read helpers + *******************************/ + + describe('getItem', () => { + it('returns the item matching the id', () => { + const sig = new Signal( + [{ id: 'a', v: 1 }, { id: 'b', v: 2 }], + { safety }, + ); + expect(sig.getItem('b')).toEqual({ id: 'b', v: 2 }); + }); + + it('returns undefined when no item matches', () => { + const sig = new Signal( + [{ id: 'a', v: 1 }], + { safety }, + ); + expect(sig.getItem('missing')).toBeUndefined(); + }); + }); + + describe('getItemIndex', () => { + it('returns the index of the matching item', () => { + const sig = new Signal( + [{ id: 'a' }, { id: 'b' }, { id: 'c' }], + { safety }, + ); + expect(sig.getItemIndex('b')).toBe(1); + }); + + it('returns -1 when no item matches', () => { + const sig = new Signal( + [{ id: 'a' }], + { safety }, + ); + expect(sig.getItemIndex('missing')).toBe(-1); + }); + }); + + /******************************* + ID utilities (pure) + *******************************/ + + describe('getID', () => { + it('reads id from any of: id, _id, hash, key', () => { + const sig = new Signal([], { safety }); + expect(sig.getID({ id: 'a' })).toBe('a'); + expect(sig.getID({ _id: 'b' })).toBe('b'); + expect(sig.getID({ hash: 'c' })).toBe('c'); + expect(sig.getID({ key: 'd' })).toBe('d'); + }); + + it('returns a string item as its own id', () => { + const sig = new Signal([], { safety }); + expect(sig.getID('plain')).toBe('plain'); + }); + }); + + describe('getIDs', () => { + it('collects all distinct ids on an item', () => { + const sig = new Signal([], { safety }); + const ids = sig.getIDs({ _id: 'one', id: 'one', key: 'two' }); + expect(ids).toContain('one'); + expect(ids).toContain('two'); + expect(ids.length).toBe(2); + }); + + it('wraps a non-object item in a one-element array', () => { + const sig = new Signal([], { safety }); + expect(sig.getIDs('plain')).toEqual(['plain']); + }); + }); + + describe('hasID', () => { + it('returns true when item carries the given id under any recognised field', () => { + const sig = new Signal([], { safety }); + expect(sig.hasID({ id: 'x' }, 'x')).toBe(true); + expect(sig.hasID({ _id: 'x' }, 'x')).toBe(true); + expect(sig.hasID({ hash: 'x' }, 'x')).toBe(true); + expect(sig.hasID({ key: 'x' }, 'x')).toBe(true); + }); + + it('returns false when the id does not match', () => { + const sig = new Signal([], { safety }); + expect(sig.hasID({ id: 'x' }, 'y')).toBe(false); + }); + }); + + /******************************* + Collection (id) mutators + *******************************/ + + describe('setArrayProperty (index form)', () => { + it('sets a property on one item by index', () => { + const sig = new Signal( + [{ id: 'a', count: 0 }, { id: 'b', count: 0 }], + { safety }, + ); + sig.setArrayProperty(0, 'count', 5); + expect(sig.raw()[0].count).toBe(5); + expect(sig.raw()[1].count).toBe(0); + }); + + it('fires reactivity', () => { + const sig = new Signal( + [{ id: 'a', count: 0 }, { id: 'b', count: 0 }], + { safety }, + ); + const sub = subscribe(sig); + sig.setArrayProperty(0, 'count', 5); + Reaction.flush(); + expect(sub.count).toBe(1); + sub.stop(); + }); + + it('preserves stored array identity (in-place mutation)', () => { + const sig = new Signal( + [{ id: 'a', count: 0 }, { id: 'b', count: 0 }], + { safety }, + ); + const before = sig.raw(); + sig.setArrayProperty(0, 'count', 5); + expect(sig.raw()).toBe(before); + }); + + it('does not fire reactivity when the value is already what was passed', () => { + const sig = new Signal( + [{ id: 'a', count: 5 }, { id: 'b', count: 0 }], + { safety }, + ); + const sub = subscribe(sig); + sig.setArrayProperty(0, 'count', 5); + Reaction.flush(); + expect(sub.count).toBe(0); + sub.stop(); + }); + }); + + describe('setArrayProperty (all-items form)', () => { + it('sets a property on every item', () => { + const sig = new Signal( + [{ id: 'a', done: false }, { id: 'b', done: false }], + { safety }, + ); + sig.setArrayProperty('done', true); + expect(sig.raw().every(item => item.done === true)).toBe(true); + }); + + it('fires reactivity', () => { + const sig = new Signal( + [{ id: 'a', done: false }, { id: 'b', done: false }], + { safety }, + ); + const sub = subscribe(sig); + sig.setArrayProperty('done', true); + Reaction.flush(); + expect(sub.count).toBe(1); + sub.stop(); + }); + + it('preserves stored array identity (in-place mutation)', () => { + const sig = new Signal( + [{ id: 'a', done: false }, { id: 'b', done: false }], + { safety }, + ); + const before = sig.raw(); + sig.setArrayProperty('done', true); + expect(sig.raw()).toBe(before); + }); + + it('does not fire reactivity when every item already has the value', () => { + const sig = new Signal( + [{ id: 'a', done: true }, { id: 'b', done: true }], + { safety }, + ); + const sub = subscribe(sig); + sig.setArrayProperty('done', true); + Reaction.flush(); + expect(sub.count).toBe(0); + sub.stop(); + }); + + it('fires reactivity when at least one item differs', () => { + const sig = new Signal( + [{ id: 'a', done: true }, { id: 'b', done: false }], + { safety }, + ); + const sub = subscribe(sig); + sig.setArrayProperty('done', true); + Reaction.flush(); + expect(sub.count).toBe(1); + sub.stop(); + }); + }); + + describe('setProperty (array form, by id)', () => { + it('sets a property on the item matching the id', () => { + const sig = new Signal( + [{ id: 'a', title: 'one' }, { id: 'b', title: 'two' }], + { safety }, + ); + sig.setProperty('b', 'title', 'TWO'); + expect(sig.raw().find(i => i.id === 'b').title).toBe('TWO'); + expect(sig.raw().find(i => i.id === 'a').title).toBe('one'); + }); + + it('fires reactivity', () => { + const sig = new Signal( + [{ id: 'a', title: 'one' }, { id: 'b', title: 'two' }], + { safety }, + ); + const sub = subscribe(sig); + sig.setProperty('b', 'title', 'TWO'); + Reaction.flush(); + expect(sub.count).toBe(1); + sub.stop(); + }); + + it('preserves stored array identity (in-place mutation)', () => { + const sig = new Signal( + [{ id: 'a', title: 'one' }, { id: 'b', title: 'two' }], + { safety }, + ); + const before = sig.raw(); + sig.setProperty('b', 'title', 'TWO'); + expect(sig.raw()).toBe(before); + }); + + it('does not fire reactivity when the field already has the value', () => { + const sig = new Signal( + [{ id: 'a', title: 'one' }, { id: 'b', title: 'two' }], + { safety }, + ); + const sub = subscribe(sig); + sig.setProperty('b', 'title', 'two'); + Reaction.flush(); + expect(sub.count).toBe(0); + sub.stop(); + }); + }); + + describe('setProperty (object form)', () => { + it('sets a property on the stored object', () => { + const sig = new Signal({ name: 'a', count: 0 }, { safety }); + sig.setProperty('count', 7); + expect(sig.raw().count).toBe(7); + }); + + it('fires reactivity', () => { + const sig = new Signal({ name: 'a', count: 0 }, { safety }); + const sub = subscribe(sig); + sig.setProperty('count', 7); + Reaction.flush(); + expect(sub.count).toBe(1); + sub.stop(); + }); + + it('preserves stored object identity (in-place mutation)', () => { + const sig = new Signal({ name: 'a', count: 0 }, { safety }); + const before = sig.raw(); + sig.setProperty('count', 7); + expect(sig.raw()).toBe(before); + }); + + it('does not fire reactivity when the field already has the value', () => { + const sig = new Signal({ name: 'a', count: 7 }, { safety }); + const sub = subscribe(sig); + sig.setProperty('count', 7); + Reaction.flush(); + expect(sub.count).toBe(0); + sub.stop(); + }); + }); + + describe('replaceItem', () => { + it('replaces the item matching the id', () => { + const sig = new Signal( + [{ id: 'a', v: 1 }, { id: 'b', v: 2 }], + { safety }, + ); + sig.replaceItem('a', { id: 'a', v: 100 }); + expect(sig.raw()[0].v).toBe(100); + expect(sig.raw()[1].v).toBe(2); + }); + + it('fires reactivity', () => { + const sig = new Signal( + [{ id: 'a', v: 1 }, { id: 'b', v: 2 }], + { safety }, + ); + const sub = subscribe(sig); + sig.replaceItem('a', { id: 'a', v: 100 }); + Reaction.flush(); + expect(sub.count).toBe(1); + sub.stop(); + }); + + it('preserves stored array identity (in-place mutation)', () => { + const sig = new Signal( + [{ id: 'a', v: 1 }, { id: 'b', v: 2 }], + { safety }, + ); + const before = sig.raw(); + sig.replaceItem('a', { id: 'a', v: 100 }); + expect(sig.raw()).toBe(before); + }); + }); + + describe('removeItem', () => { + it('removes the item matching the id', () => { + const sig = new Signal( + [{ id: 'a' }, { id: 'b' }, { id: 'c' }], + { safety }, + ); + sig.removeItem('b'); + expect(sig.raw().map(i => i.id)).toEqual(['a', 'c']); + }); + + it('fires reactivity', () => { + const sig = new Signal( + [{ id: 'a' }, { id: 'b' }, { id: 'c' }], + { safety }, + ); + const sub = subscribe(sig); + sig.removeItem('b'); + Reaction.flush(); + expect(sub.count).toBe(1); + sub.stop(); + }); + + it('preserves stored array identity (in-place mutation)', () => { + const sig = new Signal( + [{ id: 'a' }, { id: 'b' }, { id: 'c' }], + { safety }, + ); + const before = sig.raw(); + sig.removeItem('b'); + expect(sig.raw()).toBe(before); + }); + }); + + /******************************* + General mutate helper + *******************************/ + + describe('mutate', () => { + let warnSpy; + beforeEach(() => { + warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + }); + afterEach(() => { + warnSpy.mockRestore(); + }); + + it('applies side-effect mutations to the stored value', () => { + const sig = new Signal([1, 2, 3], { safety }); + sig.mutate(arr => { + arr.push(4); + }); + expect(sig.raw()).toEqual([1, 2, 3, 4]); + }); + + it('fires reactivity when the value actually changed', () => { + const sig = new Signal([1, 2, 3], { safety }); + const sub = subscribe(sig); + sig.mutate(arr => { + arr.push(4); + }); + Reaction.flush(); + expect(sub.count).toBe(1); + sub.stop(); + }); + + it('preserves stored array identity (in-place mutation)', () => { + const sig = new Signal([1, 2, 3], { safety }); + const before = sig.raw(); + sig.mutate(arr => { + arr.push(4); + }); + expect(sig.raw()).toBe(before); + }); + + it('returning a brand-new value sets it as the new value', () => { + const sig = new Signal(5, { safety }); + sig.mutate(val => val * 2); + expect(sig.raw()).toBe(10); + }); + + it('a no-op mutate does NOT re-fire subscribers', () => { + const sig = new Signal(['a', 'b'], { safety }); + const sub = subscribe(sig); + sig.mutate(() => {}); + Reaction.flush(); + expect(sub.count).toBe(0); + sub.stop(); + }); + + it('returning the same reference that was mutated in place warns in dev', () => { + const sig = new Signal([1, 2, 3], { safety }); + sig.mutate(arr => { + arr.push(4); + return arr; + }); + expect(warnSpy).toHaveBeenCalledTimes(1); + expect(warnSpy.mock.calls[0][0]).toMatch(/same reference/); + }); + }); + + /******************************* + Primitive-value helpers + *******************************/ + + describe('toggle', () => { + it('flips false to true', () => { + const sig = new Signal(false, { safety }); + sig.toggle(); + expect(sig.raw()).toBe(true); + }); + + it('flips true to false', () => { + const sig = new Signal(true, { safety }); + sig.toggle(); + expect(sig.raw()).toBe(false); + }); + + it('a pair of toggle() calls leaves the value where it started', () => { + const sig = new Signal(false, { safety }); + sig.toggle(); + sig.toggle(); + expect(sig.raw()).toBe(false); + }); + + it('fires reactivity on each flip', () => { + const sig = new Signal(false, { safety }); + const sub = subscribe(sig); + sig.toggle(); + Reaction.flush(); + sig.toggle(); + Reaction.flush(); + expect(sub.count).toBe(2); + sub.stop(); + }); + }); + + describe('increment', () => { + it('adds 1 by default', () => { + const sig = new Signal(5, { safety }); + sig.increment(); + expect(sig.raw()).toBe(6); + }); + + it('adds the given amount', () => { + const sig = new Signal(5, { safety }); + sig.increment(10); + expect(sig.raw()).toBe(15); + }); + + it('clamps to max', () => { + const sig = new Signal(8, { safety }); + sig.increment(5, 10); + expect(sig.raw()).toBe(10); + }); + + it('fires reactivity', () => { + const sig = new Signal(5, { safety }); + const sub = subscribe(sig); + sig.increment(); + Reaction.flush(); + expect(sub.count).toBe(1); + sub.stop(); + }); + }); + + describe('decrement', () => { + it('subtracts 1 by default', () => { + const sig = new Signal(5, { safety }); + sig.decrement(); + expect(sig.raw()).toBe(4); + }); + + it('subtracts the given amount', () => { + const sig = new Signal(20, { safety }); + sig.decrement(7); + expect(sig.raw()).toBe(13); + }); + + it('clamps to min', () => { + const sig = new Signal(3, { safety }); + sig.decrement(5, 0); + expect(sig.raw()).toBe(0); + }); + + it('fires reactivity', () => { + const sig = new Signal(5, { safety }); + const sub = subscribe(sig); + sig.decrement(); + Reaction.flush(); + expect(sub.count).toBe(1); + sub.stop(); + }); + }); + + describe('now', () => { + it('sets value to the current Date', () => { + const sig = new Signal(new Date(0), { safety }); + const t0 = Date.now(); + sig.now(); + const stored = sig.raw(); + expect(stored).toBeInstanceOf(Date); + expect(stored.getTime()).toBeGreaterThanOrEqual(t0); + }); + + it('fires reactivity', () => { + const sig = new Signal(new Date(0), { safety }); + const sub = subscribe(sig); + sig.now(); + Reaction.flush(); + expect(sub.count).toBe(1); + sub.stop(); + }); + }); + + describe('clear', () => { + it('sets value to undefined', () => { + const sig = new Signal({ a: 1 }, { safety }); + sig.clear(); + expect(sig.raw()).toBe(undefined); + }); + + it('fires reactivity', () => { + const sig = new Signal({ a: 1 }, { safety }); + const sub = subscribe(sig); + sig.clear(); + Reaction.flush(); + expect(sub.count).toBe(1); + sub.stop(); + }); + }); + + /******************************* + Type-mismatched helper calls + — helpers are not type-guarded; + these tests document actual behaviour + so the contract is explicit for users + *******************************/ + + describe('type-mismatched helper calls', () => { + it('toggle() on a number coerces via NOT — truthy becomes false', () => { + const sig = new Signal(1, { safety }); + sig.toggle(); + expect(sig.raw()).toBe(false); + }); + + it('toggle() on undefined flips to true', () => { + const sig = new Signal(undefined, { safety }); + sig.toggle(); + expect(sig.raw()).toBe(true); + }); + + it('increment() on a string concatenates rather than throwing', () => { + const sig = new Signal('count', { safety }); + sig.increment(1); + expect(sig.raw()).toBe('count1'); + }); + + it('push() on a non-array signal throws', () => { + const sig = new Signal(42, { safety }); + expect(() => sig.push('x')).toThrow(); + }); + }); + + /******************************* + Safety contract — clone mode + does not leak the live ref + *******************************/ + + if (safety === 'clone') { + describe('safety contract: stored value is shielded from external mutation', () => { + it('peek() returns a copy — mutating it does not affect the signal', () => { + const sig = new Signal({ count: 0 }, { safety }); + const copy = sig.peek(); + copy.count = 999; + expect(sig.raw().count).toBe(0); + }); + + it('get() returns a copy — mutating it does not affect the signal', () => { + const sig = new Signal([1, 2, 3], { safety }); + const copy = sig.get(); + copy.push(999); + expect(sig.raw()).toEqual([1, 2, 3]); + }); + + it('after a helper mutation, external reads still get a copy', () => { + const sig = new Signal([{ id: 'a', v: 1 }], { safety }); + sig.push({ id: 'b', v: 2 }); + const copy = sig.peek(); + copy.push({ id: 'c', v: 3 }); + expect(sig.raw().length).toBe(2); + }); + }); + } + }); +}); diff --git a/packages/reactivity/test/unit/signal.test.js b/packages/reactivity/test/unit/signal.test.js index a76aa09ff..7d06f26e6 100644 --- a/packages/reactivity/test/unit/signal.test.js +++ b/packages/reactivity/test/unit/signal.test.js @@ -248,289 +248,10 @@ describe.concurrent('Signal', () => { }); }); - /******************************* - Array - *******************************/ - - describe.concurrent('Array Utilities', () => { - it('Push should handle multiple values', () => { - const reactiveArray = new Signal([1]); - reactiveArray.push(2, 3, 4); - expect(reactiveArray.value).toEqual([1, 2, 3, 4]); - }); - - it('Unshift should handle multiple values', () => { - const reactiveArray = new Signal([4]); - reactiveArray.unshift(1, 2, 3); - expect(reactiveArray.value).toEqual([1, 2, 3, 4]); - }); - - it('Splice should remove and insert elements', () => { - const reactiveArray = new Signal(['a', 'b', 'c', 'd']); - reactiveArray.splice(1, 1, 'x'); - expect(reactiveArray.value).toEqual(['a', 'x', 'c', 'd']); - }); - - it('Splice should handle multiple inserts', () => { - const reactiveArray = new Signal(['a', 'b', 'c']); - reactiveArray.splice(1, 0, 'x', 'y'); - expect(reactiveArray.value).toEqual(['a', 'x', 'y', 'b', 'c']); - }); - - it('Splice should handle deletion without insertion', () => { - const reactiveArray = new Signal(['a', 'b', 'c']); - reactiveArray.splice(1, 2); - expect(reactiveArray.value).toEqual(['a']); - }); - - it('Splice should trigger reactions when modified', () => { - const callback = vi.fn(); - const reactiveArray = new Signal(['a', 'b', 'c']); - - Reaction.create(() => { - callback(reactiveArray.get()); - }); - - reactiveArray.splice(1, 1, 'x'); - Reaction.flush(); - - expect(callback).toHaveBeenCalledTimes(2); - expect(callback).toHaveBeenLastCalledWith(['a', 'x', 'c']); - }); - - it('setIndex should change value at index', () => { - const reactiveArray = new Signal([1, 2, 3]); - reactiveArray.setIndex(1, 'two'); // Change value at index 1 to 'two' - expect(reactiveArray.value).toEqual([1, 'two', 3]); - }); - - it('removeIndex should remove value at index', () => { - const reactiveArray = new Signal([1, 2, 3]); - reactiveArray.removeIndex(1); // Remove value at index 1 - expect(reactiveArray.value).toEqual([1, 3]); - }); - - it('setArrayProperty should set an object property at index', () => { - const reactiveArray = new Signal([{ name: 'Alice' }, { name: 'Bob' }]); - reactiveArray.setArrayProperty(1, 'name', 'Charlie'); // Change name of the second object - expect(reactiveArray.value).toEqual([{ name: 'Alice' }, { name: 'Charlie' }]); - }); - - it('setArrayProperty should set all object properties when no index specified', () => { - const reactiveArray = new Signal([{ name: 'Alice' }, { name: 'Bob' }]); - reactiveArray.setArrayProperty('status', 'active'); // Set 'status' property for all objects - expect(reactiveArray.value).toEqual([ - { name: 'Alice', status: 'active' }, - { name: 'Bob', status: 'active' }, - ]); - }); - }); - - describe.concurrent('Transformation Helpers', () => { - it('map should transform each item in the array', () => { - const numbers = new Signal([1, 2, 3]); - numbers.map(num => num * 2); - expect(numbers.get()).toEqual([2, 4, 6]); - }); - - it('map should trigger reactions', () => { - const callback = vi.fn(); - const numbers = new Signal([1, 2, 3]); - - Reaction.create(() => { - callback(numbers.get()); - }); - - numbers.map(num => num * 2); - Reaction.flush(); - - expect(callback).toHaveBeenCalledTimes(2); - expect(callback).toHaveBeenLastCalledWith([2, 4, 6]); - }); - - it('filter should remove items based on predicate', () => { - const numbers = new Signal([1, 2, 3, 4, 5]); - numbers.filter(num => num % 2 === 1); - expect(numbers.get()).toEqual([1, 3, 5]); - }); - - it('filter should trigger reactions', () => { - const callback = vi.fn(); - const numbers = new Signal([1, 2, 3, 4, 5]); - - Reaction.create(() => { - callback(numbers.get()); - }); - - numbers.filter(num => num > 3); - Reaction.flush(); - - expect(callback).toHaveBeenCalledTimes(2); - expect(callback).toHaveBeenLastCalledWith([4, 5]); - }); - - it('filter should handle complex objects', () => { - const items = new Signal([ - { id: 1, active: true }, - { id: 2, active: false }, - { id: 3, active: true }, - ]); - - items.filter(item => item.active); - expect(items.get()).toEqual([ - { id: 1, active: true }, - { id: 3, active: true }, - ]); - }); - - it('map should handle complex transformations', () => { - const items = new Signal([ - { id: 1, value: 10 }, - { id: 2, value: 20 }, - ]); - - items.map(item => ({ - ...item, - doubled: item.value * 2, - })); - - expect(items.get()).toEqual([ - { id: 1, value: 10, doubled: 20 }, - { id: 2, value: 20, doubled: 40 }, - ]); - }); - }); - - describe.concurrent('Boolean Helpers', () => { - it('toggle should toggle a boolean', () => { - const reactiveBool = new Signal(true); - reactiveBool.toggle(); - expect(reactiveBool.value).toBe(false); - reactiveBool.toggle(); - expect(reactiveBool.value).toBe(true); - }); - }); - - describe.concurrent('Mutation Utilities', () => { - it('map should apply a transformation to each item', () => { - const numbers = new Signal([1, 2, 3]); - numbers.map(num => num * 2); - expect(numbers.get()).toEqual([2, 4, 6]); - }); - - it('filter should remove items based on a filter', () => { - const numbers = new Signal([1, 2, 3, 4, 5]); - numbers.filter(num => num % 2 === 1); // Remove even numbers - expect(numbers.get()).toEqual([1, 3, 5]); - }); - }); - - describe.concurrent('ID Utilities', () => { - it('getID should get id from an item', () => { - const id1 = 'one'; - const id2 = { _id: 'one' }; - const id3 = { id: 'one' }; - const id4 = { hash: 'one' }; - const id5 = { key: 'one' }; - - const signal = new Signal(); - expect(signal.getID(id1)).toBe('one'); - expect(signal.getID(id2)).toBe('one'); - expect(signal.getID(id3)).toBe('one'); - expect(signal.getID(id4)).toBe('one'); - expect(signal.getID(id5)).toBe('one'); - }); - - it('getIDs should get all ids from an item', () => { - const item = { _id: 'one', id: 'one', key: 'two' }; - const signal = new Signal(); - const ids = signal.getIDs(item); - expect(ids).toContain('one'); - expect(ids).toContain('two'); - expect(ids.length).toEqual(2); - }); - - it('hasID should match an item ID', () => { - const item = { _id: 'one' }; - const signal = new Signal(); - expect(signal.hasID(item, 'one')).toEqual(true); - }); - }); - - describe.concurrent('ID Helpers', () => { - // need separate copy for each test - const arrayItems = () => [ - { id: 1, name: 'Item 1' }, - { id: 2, name: 'Item 2' }, - ]; - - it('setProperty should set the property of the item matching an id', () => { - const items = new Signal(arrayItems()); - items.setProperty(1, 'name', 'Updated Item 1'); - expect(items.get()).toEqual([ - { id: 1, name: 'Updated Item 1' }, - { id: 2, name: 'Item 2' }, - ]); - }); - - it('getItemIndex should get the item index with matching id', () => { - const items = new Signal(arrayItems()); - const index = items.getItemIndex(2); - expect(index).toBe(1); - }); - - it('getItem should get the item with matching id', () => { - const items = new Signal(arrayItems()); - const item = items.getItem(2); - expect(item).toEqual({ id: 2, name: 'Item 2' }); - }); - - it('replaceItem should replace an item matching an ID', () => { - const items = new Signal(arrayItems()); - items.replaceItem(1, { id: 1, name: 'Replaced Item 1' }); - expect(items.get()).toEqual([ - { id: 1, name: 'Replaced Item 1' }, - { id: 2, name: 'Item 2' }, - ]); - }); - - it('removeItem should remove an item matching an ID', () => { - const items = new Signal(arrayItems()); - items.removeItem(1); - expect(items.get()).toEqual([ - { id: 2, name: 'Item 2' }, - ]); - }); - - it('setProperty should set the property of the item matching a given id', () => { - const items = new Signal(arrayItems()); - items.setProperty(2, 'status', 'active'); - expect(items.get()).toEqual([ - { id: 1, name: 'Item 1' }, - { id: 2, name: 'Item 2', status: 'active' }, - ]); - }); - - it('setArrayProperty should set an object property at index', () => { - const items = new Signal(arrayItems()); - items.setArrayProperty(1, 'status', 'pending'); - expect(items.get()[1].status).toBe('pending'); - }); - - it('setArrayProperty should set all object properties when no index specified', () => { - const items = new Signal(arrayItems()); - items.setArrayProperty('status', 'active'); - expect(items.get()).toEqual([ - { id: 1, name: 'Item 1', status: 'active' }, - { id: 2, name: 'Item 2', status: 'active' }, - ]); - }); - }); - describe.concurrent('Cloning Behavior with Signals', () => { it('should maintain reactivity when using a Signal inside another Signal', () => { const innerCallback = vi.fn(); - const innerVar = new Signal(1, { allowClone: true }); + const innerVar = new Signal(1, { safety: 'clone' }); const outerCallback = vi.fn(); const outerVar = new Signal(innerVar); @@ -555,8 +276,8 @@ describe.concurrent('Signal', () => { const data1 = { id: 1, text: 'test object' }; const data2 = { id: 2, text: 'test object 2' }; - const innerVar1 = new Signal(data1, { allowClone: true }); - const innerVar2 = new Signal(data2, { allowClone: true }); + const innerVar1 = new Signal(data1, { safety: 'clone' }); + const innerVar2 = new Signal(data2, { safety: 'clone' }); innerVar1.subscribe(innerCallback1); innerVar2.subscribe(innerCallback2); @@ -787,7 +508,7 @@ describe.concurrent('Signal', () => { obj => ({ doubled: obj.count * 2 }), { equalityFunction: (a, b) => a?.doubled === b?.doubled, - allowClone: false, + safety: 'reference', }, ); @@ -954,26 +675,6 @@ describe.concurrent('Signal', () => { expect(b.dependency.subscribers.size).toBe(1); }); }); - - /******************************* - Symbol.hasInstance (instanceof) - *******************************/ - - describe.concurrent('instanceof Signal', () => { - it('should return true for Signal instances', () => { - expect(new Signal(0) instanceof Signal).toBe(true); - }); - - it('should return false for non-Signal objects', () => { - expect({} instanceof Signal).toBe(false); - expect(null instanceof Signal).toBe(false); - }); - - it('should return true for objects created via Object.create(Signal.prototype)', () => { - const fake = Object.create(Signal.prototype); - expect(fake instanceof Signal).toBe(true); - }); - }); }); describe('Signal API', () => { @@ -1008,9 +709,9 @@ describe('Signal API', () => { expect(signal.peek().count).toBe(0); }); - it('does NOT clone when allowClone is false — external mutation reflects through peek()', () => { + it('permits mutations through peek when using no safety', () => { const original = { count: 0 }; - const signal = new Signal(original, { allowClone: false }); + const signal = new Signal(original, { safety: 'reference' }); original.count = 99; expect(signal.peek().count).toBe(99); }); @@ -1046,7 +747,7 @@ describe('Signal API', () => { }); /*********************************************** - * get() / value getter — reactive read with cloning + * get() / value getter ***********************************************/ describe('get() and value', () => { @@ -1061,14 +762,6 @@ describe('Signal API', () => { expect(cb).toHaveBeenCalledTimes(2); }); - it('returns a cloned object each call so callers cannot poison internal state by mutating the result', () => { - const signal = new Signal({ inner: 'untouched' }); - const a = signal.get(); - a.inner = 'mutated'; - // Second get returns a fresh clone — first mutation did not stick - expect(signal.get().inner).toBe('untouched'); - }); - it('value getter is an alias for get() — both produce the same value', () => { const counter = new Signal(42); expect(counter.value).toBe(counter.get()); @@ -1161,7 +854,7 @@ describe('Signal API', () => { }); /*********************************************** - * subscribe() — callback observer that fires once synchronously + on changes + * subscribe() — callback observer ***********************************************/ describe('subscribe()', () => { @@ -1203,44 +896,6 @@ describe('Signal API', () => { }); }); - /*********************************************** - * clear() — set to undefined - ***********************************************/ - - describe('clear()', () => { - it('sets the value to undefined', () => { - const signal = new Signal('something'); - signal.clear(); - expect(signal.get()).toBeUndefined(); - }); - - it('triggers subscribers when clearing a non-undefined value', () => { - const callback = vi.fn(); - const signal = new Signal({ name: 'Alice' }); - signal.subscribe(callback); - Reaction.flush(); - expect(callback).toHaveBeenCalledTimes(1); - - signal.clear(); - Reaction.flush(); - expect(callback).toHaveBeenCalledTimes(2); - expect(callback.mock.calls.at(-1)[0]).toBeUndefined(); - }); - - it('does NOT re-fire subscribers when clearing an already-undefined signal', () => { - const callback = vi.fn(); - const signal = new Signal(undefined); - signal.subscribe(callback); - Reaction.flush(); - expect(callback).toHaveBeenCalledTimes(1); - - signal.clear(); - Reaction.flush(); - // Equality blocks the no-op - expect(callback).toHaveBeenCalledTimes(1); - }); - }); - /*********************************************** * notify() — force-fire bypassing equality ***********************************************/ @@ -1249,13 +904,13 @@ describe('Signal API', () => { it('force-triggers subscribers even when the value reference is identical', () => { // mutate the underlying object via peek(), then notify() so subscribers re-run. const callback = vi.fn(); - const data = new Signal({ count: 0 }, { allowClone: false }); + const data = new Signal({ count: 0 }); Reaction.create(() => callback(data.get().count)); Reaction.flush(); expect(callback).toHaveBeenCalledTimes(1); expect(callback).toHaveBeenLastCalledWith(0); - data.peek().count = 5; + data.raw().count = 5; data.notify(); Reaction.flush(); expect(callback).toHaveBeenCalledTimes(2); @@ -1323,404 +978,6 @@ describe('Signal API', () => { }); }); - /*********************************************** - * mutate() — safe in-place mutation - ***********************************************/ - - describe('mutate()', () => { - let warnSpy; - beforeEach(() => { - warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); - }); - afterEach(() => { - warnSpy.mockRestore(); - }); - - it('in-place mutation without returning fires subscribers when the value changed', () => { - const callback = vi.fn(); - const items = new Signal(['apple', 'banana']); - items.subscribe(callback); - Reaction.flush(); - expect(callback).toHaveBeenCalledTimes(1); - - items.mutate(arr => { - arr.push('orange'); - }); - Reaction.flush(); - expect(callback).toHaveBeenCalledTimes(2); - expect(items.get()).toEqual(['apple', 'banana', 'orange']); - }); - - it('returning a brand-new value from the mutation function sets it as the new value', () => { - const count = new Signal(5); - count.mutate(val => val * 2); - expect(count.get()).toBe(10); - }); - - it('in-place mutation that does not actually change the value does NOT re-fire subscribers', () => { - const callback = vi.fn(); - const items = new Signal(['a', 'b']); - items.subscribe(callback); - Reaction.flush(); - expect(callback).toHaveBeenCalledTimes(1); - - // mutate that touches nothing — equality says no-change - items.mutate(() => {}); - Reaction.flush(); - expect(callback).toHaveBeenCalledTimes(1); - }); - - it('returning the same reference that was mutated in place warns in dev', () => { - const signal = new Signal([1, 2, 3]); - signal.mutate(arr => { - arr.push(4); - return arr; - }); - expect(warnSpy).toHaveBeenCalledTimes(1); - expect(warnSpy.mock.calls[0][0]).toMatch(/same reference/); - }); - }); - - /*********************************************** - * Array push / unshift / splice — mutating helpers that bypass equality - ***********************************************/ - - describe('push() / unshift() / splice() — array helpers', () => { - it('push() appends a single item and triggers subscribers', () => { - const callback = vi.fn(); - const notifications = new Signal([{ text: 'Welcome!', read: false }]); - notifications.subscribe(callback); - Reaction.flush(); - expect(callback).toHaveBeenCalledTimes(1); - - notifications.push({ text: 'New message', read: false }); - Reaction.flush(); - expect(callback).toHaveBeenCalledTimes(2); - expect(notifications.get()).toHaveLength(2); - expect(notifications.get()[1].text).toBe('New message'); - }); - - it('push() accepts multiple arguments and appends them in order', () => { - const items = new Signal([1]); - items.push(2, 3, 4); - expect(items.get()).toEqual([1, 2, 3, 4]); - }); - - it('unshift() prepends items so the new value lands at the front', () => { - const callback = vi.fn(); - const messages = new Signal([{ user: 'system', text: 'started' }]); - messages.subscribe(callback); - Reaction.flush(); - - messages.unshift({ user: 'Alice', text: 'Hello!' }); - Reaction.flush(); - expect(messages.get()[0].user).toBe('Alice'); - expect(callback).toHaveBeenCalledTimes(2); - }); - - it('splice() can insert without removing any items', () => { - const playlist = new Signal(['t1', 'New', 't3', 't4']); - playlist.splice(2, 0, 'Bonus 1', 'Bonus 2'); - expect(playlist.get()).toEqual(['t1', 'New', 'Bonus 1', 'Bonus 2', 't3', 't4']); - }); - - it('splice() removing the entire array yields an empty array', () => { - const items = new Signal(['a', 'b', 'c']); - items.splice(0, 3); - expect(items.get()).toEqual([]); - }); - }); - - /*********************************************** - * map() / filter() — in-place transformation helpers - ***********************************************/ - - describe('map() / filter() — transformation helpers', () => { - it('map() replaces the array in place with the transformed values', () => { - const prices = new Signal([10, 20, 30, 40]); - prices.map(price => price * 0.9); - expect(prices.get()).toEqual([9, 18, 27, 36]); - }); - - it('filter() keeps only items that match the predicate', () => { - const numbers = new Signal([1, 2, 3, 4, 5, 6]); - numbers.filter(n => n % 2 === 0); - expect(numbers.get()).toEqual([2, 4, 6]); - }); - - it('filter() to an empty result yields an empty array (not undefined)', () => { - const items = new Signal([1, 2, 3]); - items.filter(() => false); - expect(items.get()).toEqual([]); - }); - - it('map() and filter() both trigger subscribers on transformation', () => { - const callback = vi.fn(); - const items = new Signal([1, 2, 3]); - items.subscribe(callback); - Reaction.flush(); - - items.map(x => x + 1); - Reaction.flush(); - expect(callback).toHaveBeenCalledTimes(2); - - items.filter(x => x > 2); - Reaction.flush(); - expect(callback).toHaveBeenCalledTimes(3); - expect(items.get()).toEqual([3, 4]); - }); - }); - - /*********************************************** - * Index helpers — getIndex / setIndex / removeIndex - ***********************************************/ - - describe('getIndex() / setIndex() / removeIndex()', () => { - it('getIndex() reads the item at a given position', () => { - const colors = new Signal(['red', 'green', 'blue']); - expect(colors.getIndex(0)).toBe('red'); - expect(colors.getIndex(2)).toBe('blue'); - }); - - it('getIndex() creates a dependency, so writes via setIndex() re-fire reactions', () => { - const colors = new Signal(['red', 'green', 'blue', 'yellow']); - const fn = vi.fn(); - Reaction.create(() => fn(colors.getIndex(0))); - expect(fn).toHaveBeenCalledTimes(1); - expect(fn).toHaveBeenLastCalledWith('red'); - - colors.setIndex(0, 'purple'); - Reaction.flush(); - expect(fn).toHaveBeenCalledTimes(2); - expect(fn).toHaveBeenLastCalledWith('purple'); - }); - - it('setIndex() replaces the item at the given index', () => { - const letters = new Signal(['a', 'b', 'c']); - letters.setIndex(1, 'x'); - expect(letters.get()).toEqual(['a', 'x', 'c']); - }); - - it('removeIndex() drops the item at the given index and shortens the array', () => { - const tasks = new Signal([ - { text: 'Learn', completed: true }, - { text: 'Write', completed: false }, - ]); - tasks.removeIndex(0); - expect(tasks.get()).toEqual([{ text: 'Write', completed: false }]); - }); - }); - - /*********************************************** - * setArrayProperty — single-index or all-items property set - ***********************************************/ - - describe('setArrayProperty()', () => { - it('with three arguments, sets the property on the object at the given index only', () => { - const tasks = new Signal([ - { id: 1, title: 'Buy groceries', completed: false }, - { id: 2, title: 'Walk the dog', completed: false }, - ]); - tasks.setArrayProperty(0, 'completed', true); - expect(tasks.get()).toEqual([ - { id: 1, title: 'Buy groceries', completed: true }, - { id: 2, title: 'Walk the dog', completed: false }, - ]); - }); - - it('with two arguments, sets the property on every object in the array', () => { - const tasks = new Signal([ - { id: 1, completed: false }, - { id: 2, completed: false }, - { id: 3, completed: false }, - ]); - tasks.setArrayProperty('completed', true); - expect(tasks.get().every(t => t.completed === true)).toBe(true); - }); - }); - - /*********************************************** - * ID-based collection helpers - ***********************************************/ - - describe('ID-based helpers', () => { - const users = () => [ - { id: 'user1', name: 'Alice', status: 'online' }, - { id: 'user2', name: 'Bob', status: 'offline' }, - { id: 'user3', name: 'Carol', status: 'online' }, - ]; - - it('getItem() locates an object by its id field', () => { - const signal = new Signal(users()); - expect(signal.getItem('user2')).toEqual({ id: 'user2', name: 'Bob', status: 'offline' }); - }); - - it('getItem() returns undefined when no item matches the id', () => { - const signal = new Signal(users()); - expect(signal.getItem('missing')).toBeUndefined(); - }); - - it('getItemIndex() returns the index of the matching item, or -1 when not found', () => { - const signal = new Signal(users()); - expect(signal.getItemIndex('user2')).toBe(1); - expect(signal.getItemIndex('unknown')).toBe(-1); - }); - - it('replaceItem() swaps the entire object matching the id while leaving others untouched', () => { - const contacts = new Signal([ - { id: 'alice123', name: 'Alice', email: 'alice@email.com' }, - { id: 'bob456', name: 'Bob', email: 'bob@email.com' }, - ]); - const updated = { id: 'alice123', name: 'Alice Smith', email: 'asmith@email.com' }; - contacts.replaceItem('alice123', updated); - expect(contacts.get()[0]).toEqual(updated); - expect(contacts.get()[1].name).toBe('Bob'); - }); - - it('removeItem() removes the matching object and leaves the rest in order', () => { - const signal = new Signal(users()); - signal.removeItem('user2'); - expect(signal.get().map(u => u.id)).toEqual(['user1', 'user3']); - }); - - it('setProperty(id, field, value) on an array signal updates the named field on the matching item', () => { - const signal = new Signal(users()); - signal.setProperty('user2', 'status', 'online'); - expect(signal.getItem('user2').status).toBe('online'); - expect(signal.getItem('user1').status).toBe('online'); // others untouched - }); - - it('setProperty(field, value) on an object signal updates one field of that object', () => { - const user = new Signal({ name: 'Alice', age: 30 }); - user.setProperty('name', 'Bob'); - expect(user.get()).toEqual({ name: 'Bob', age: 30 }); - }); - - it('recognises id from any of: id, _id, hash, key fields', () => { - const signal = new Signal(); - expect(signal.getID({ id: 'a' })).toBe('a'); - expect(signal.getID({ _id: 'b' })).toBe('b'); - expect(signal.getID({ hash: 'c' })).toBe('c'); - expect(signal.getID({ key: 'd' })).toBe('d'); - }); - - it('hasID() returns true when the item carries that id under any recognised field', () => { - const signal = new Signal(); - expect(signal.hasID({ id: 'x' }, 'x')).toBe(true); - expect(signal.hasID({ _id: 'x' }, 'x')).toBe(true); - expect(signal.hasID({ key: 'x' }, 'x')).toBe(true); - expect(signal.hasID({ id: 'x' }, 'y')).toBe(false); - }); - }); - - /*********************************************** - * toggle() — boolean flip - ***********************************************/ - - describe('toggle()', () => { - it('flips false to true', () => { - const flag = new Signal(false); - flag.toggle(); - expect(flag.get()).toBe(true); - }); - - it('flips true to false', () => { - const flag = new Signal(true); - flag.toggle(); - expect(flag.get()).toBe(false); - }); - - it('a pair of toggle() calls leaves the value where it started', () => { - const flag = new Signal(false); - flag.toggle(); - flag.toggle(); - expect(flag.get()).toBe(false); - }); - - it('triggers subscribers on each flip', () => { - const callback = vi.fn(); - const flag = new Signal(false); - flag.subscribe(callback); - Reaction.flush(); - flag.toggle(); - Reaction.flush(); - flag.toggle(); - Reaction.flush(); - expect(callback).toHaveBeenCalledTimes(3); - }); - }); - - /*********************************************** - * increment() / decrement() - ***********************************************/ - - describe('increment() / decrement()', () => { - it('increment() with no argument adds 1', () => { - const counter = new Signal(0); - counter.increment(); - expect(counter.get()).toBe(1); - }); - - it('increment(amount) adds the given amount', () => { - const counter = new Signal(0); - counter.increment(10); - expect(counter.get()).toBe(10); - }); - - it('increment(amount, max) caps the result at max', () => { - const counter = new Signal(95); - counter.increment(10, 100); - expect(counter.get()).toBe(100); - }); - - it('decrement() with no argument subtracts 1', () => { - const score = new Signal(100); - score.decrement(); - expect(score.get()).toBe(99); - }); - - it('decrement(amount) subtracts the given amount', () => { - const score = new Signal(100); - score.decrement(10); - expect(score.get()).toBe(90); - }); - - it('decrement(amount, min) floors the result at min', () => { - const score = new Signal(5); - score.decrement(10, 0); - expect(score.get()).toBe(0); - }); - }); - - /*********************************************** - * now() — set to current Date - ***********************************************/ - - describe('now()', () => { - it('replaces the value with a fresh Date instance', () => { - const lastUpdated = new Signal(new Date(0)); - lastUpdated.now(); - const stored = lastUpdated.get(); - expect(stored).toBeInstanceOf(Date); - // The new timestamp should be strictly later than the epoch we seeded - expect(stored.getTime()).toBeGreaterThan(0); - }); - - it('triggers subscribers when called', async () => { - const callback = vi.fn(); - const lastUpdated = new Signal(new Date(0)); - lastUpdated.subscribe(callback); - Reaction.flush(); - expect(callback).toHaveBeenCalledTimes(1); - - // Spacing to guarantee a different millisecond timestamp - await new Promise(r => setTimeout(r, 2)); - lastUpdated.now(); - Reaction.flush(); - expect(callback).toHaveBeenCalledTimes(2); - }); - }); - /*********************************************** * derive() — single-source transformation ***********************************************/ @@ -1819,41 +1076,10 @@ describe('Signal API', () => { expect(undefined instanceof Signal).toBe(false); expect(42 instanceof Signal).toBe(false); }); - }); - - /*********************************************** - * Negative path coverage — type misuse on helpers - * The framework documents helpers per data type (push for arrays, - * toggle for booleans, etc.). When a user calls a helper that - * doesn't apply to the signal's value, what happens? - * These tests document the actual behaviour as the user-visible contract. - ***********************************************/ - - describe('Type-mismatched helper calls', () => { - it('toggle() on a non-boolean coerces via NOT — numbers become false', () => { - // !1 = false - const signal = new Signal(1); - signal.toggle(); - expect(signal.get()).toBe(false); - }); - - it('toggle() on undefined becomes true', () => { - const signal = new Signal(undefined); - signal.toggle(); - expect(signal.get()).toBe(true); - }); - it('increment() on a string concatenates — numeric helpers are not type-guarded', () => { - // uses + operator without coercion - const signal = new Signal('count'); - signal.increment(1); - expect(signal.get()).toBe('count1'); - }); - - it('push() on a non-array signal throws because the underlying value has no push method', () => { - // calls this.currentValue.push directly - const signal = new Signal(42); - expect(() => signal.push('x')).toThrow(); + it('matches objects created via Object.create(Signal.prototype)', () => { + const fake = Object.create(Signal.prototype); + expect(fake instanceof Signal).toBe(true); }); }); }); diff --git a/packages/renderer/src/engines/lit/renderer.js b/packages/renderer/src/engines/lit/renderer.js index 3e312e5fb..1391ba1ed 100644 --- a/packages/renderer/src/engines/lit/renderer.js +++ b/packages/renderer/src/engines/lit/renderer.js @@ -57,7 +57,7 @@ export class LitRenderer { this.inheritsData = inheritsData; // for subtrees lets us know if this needs to have data updates downstream this.protectedKeys = protectedKeys; // keys scoped to this subtree (loop vars, async results) that parent updates cannot overwrite this.id = LitRenderer.getID({ ast, data, isSVG }); - this.dataVersion = new Signal(0, { allowClone: false, equalityFunction: () => false }); + this.dataVersion = new Signal(0, { safety: 'none' }); // Delegate expression evaluation this.evaluator = new ExpressionEvaluator({ diff --git a/packages/renderer/src/engines/native/reactive-context.js b/packages/renderer/src/engines/native/reactive-context.js index 3c0d8ecc7..d24b485fb 100644 --- a/packages/renderer/src/engines/native/reactive-context.js +++ b/packages/renderer/src/engines/native/reactive-context.js @@ -28,8 +28,7 @@ import { UNWRAP } from '../../helpers.js'; get method. We deliberately do not allocate a full Signal per key: Signal wraps - a Dependency with allowClone / equalityFunction / clone / currentValue - field assignments per instance. The wrapper allocation dominates at + a Dependency. The wrapper allocation dominates at scale where many records each carry several keys. Equality dedup is preserved: `Signal.equalityFunction` is snapshotted at construction, matching Signal's per-instance snapshot semantics so the inlined diff --git a/packages/renderer/test/browser/subtree-spurious.test.js b/packages/renderer/test/browser/subtree-spurious.test.js index 73f502ee3..018f6a024 100644 --- a/packages/renderer/test/browser/subtree-spurious.test.js +++ b/packages/renderer/test/browser/subtree-spurious.test.js @@ -986,7 +986,7 @@ RENDERING_ENGINES.forEach(engine => { tagName: tag, template: '{#each fruit in fruits}{capture fruit}{/each}', createComponent: ({ signal }) => { - const fruits = signal([ref], { allowClone: false }); + const fruits = signal([ref], { safety: 'reference' }); return { fruits, capture: (fruit) => { @@ -1017,7 +1017,7 @@ RENDERING_ENGINES.forEach(engine => { tagName: tag, template: '{#each fruit in fruits}{capture(fruit)}{/each}', createComponent: ({ signal }) => { - const fruits = signal([ref], { allowClone: false }); + const fruits = signal([ref], { safety: 'reference' }); return { fruits, capture: (fruit) => { @@ -1047,7 +1047,7 @@ RENDERING_ENGINES.forEach(engine => { tagName: tag, template: '{#each d in dates}{readTime d}{/each}', createComponent: ({ signal }) => { - const dates = signal([new Date('2024-01-01T00:00:00Z')], { allowClone: false }); + const dates = signal([new Date('2024-01-01T00:00:00Z')], { safety: 'reference' }); return { dates, readTime: (d) => { @@ -1079,7 +1079,7 @@ RENDERING_ENGINES.forEach(engine => { tagName: tag, template: '{#each fruit in fruits}{readCache fruit}{/each}', createComponent: ({ signal }) => { - const fruits = signal([ref], { allowClone: false }); + const fruits = signal([ref], { safety: 'reference' }); return { fruits, readCache: (fruit) => { diff --git a/packages/templating/src/template.js b/packages/templating/src/template.js index 78efc4e3a..c099b0039 100644 --- a/packages/templating/src/template.js +++ b/packages/templating/src/template.js @@ -1001,7 +1001,7 @@ export const Template = class Template { if (property in target) { let signal = template.settingsVars.get(property); if (!signal) { - signal = new Signal(target[property], { allowClone: false }); + signal = new Signal(target[property], { safety: 'reference' }); template.settingsVars.set(property, signal); } signal.get(); // track dependency @@ -1019,7 +1019,7 @@ export const Template = class Template { signal.set(value); } else { - signal = new Signal(value, { allowClone: false }); + signal = new Signal(value, { safety: 'reference' }); template.settingsVars.set(property, signal); } return true; diff --git a/packages/templating/test/data-context.test.js b/packages/templating/test/data-context.test.js index 281d250be..03c311e50 100644 --- a/packages/templating/test/data-context.test.js +++ b/packages/templating/test/data-context.test.js @@ -45,7 +45,7 @@ describe('Template — createReactiveState', () => { defaultState: { config: { value: { x: 1 }, - options: { equalityFunction: strictEquality, allowClone: false }, + options: { equalityFunction: strictEquality, safety: 'clone' }, }, }, }); diff --git a/packages/templating/test/subtemplate-settings.test.js b/packages/templating/test/subtemplate-settings.test.js index 64135fbf8..e055f6fc2 100644 --- a/packages/templating/test/subtemplate-settings.test.js +++ b/packages/templating/test/subtemplate-settings.test.js @@ -487,7 +487,7 @@ describe('Subtemplate settings reactivity', () => { it('does not trigger reactivity when the underlying object is mutated in place', () => { // Consumers must REPLACE values via Proxy set, not mutate the stored - // object in place. allowClone:false stores by reference, and the Signal's + // object in place. safety: 'reference' stores by reference, and the Signal's // deep-equality on set() makes a structurally identical replacement after // a mutation a no-op. const { child } = makeSubtemplatePair({ diff --git a/packages/utils/src/functions.js b/packages/utils/src/functions.js index 720efc674..8fefdd642 100755 --- a/packages/utils/src/functions.js +++ b/packages/utils/src/functions.js @@ -15,6 +15,11 @@ export const noop = () => {}; */ export const identity = (v) => v; +export const returnsTrue = () => true; +export const returnsFalse = () => false; +export const returnsSelf = identity; +export const returnsUndefined = noop; + /* Call function even if its not defined */ diff --git a/tools/benchmark/src/main.js b/tools/benchmark/src/main.js index 2ba93247c..e96f50f5f 100644 --- a/tools/benchmark/src/main.js +++ b/tools/benchmark/src/main.js @@ -5,8 +5,8 @@ import { buildData } from './store.js'; import ast from './template.html?ast'; const defaultState = { - rows: { value: [], options: { allowClone: false, equalityFunction: () => false } }, - selected: { value: 0, options: { allowClone: false, equalityFunction: () => false } }, + rows: { value: [], options: { safety: 'none' } }, + selected: { value: 0, options: { safety: 'none' } }, }; const createComponent = ({ state }) => ({ From 0dd3f4f05b1bbc632e9356b45b92ebdd0ce2e689 Mon Sep 17 00:00:00 2001 From: Jack Date: Mon, 18 May 2026 16:01:16 -0400 Subject: [PATCH 02/19] Feat: Swap to reference safety by default. --- packages/reactivity/src/signal.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/reactivity/src/signal.js b/packages/reactivity/src/signal.js index 34845692c..111a72890 100755 --- a/packages/reactivity/src/signal.js +++ b/packages/reactivity/src/signal.js @@ -29,7 +29,7 @@ export class Signal { // default clone and equal pulled from utils static equalityFunction = isEqual; static cloneFunction = clone; - static safety = 'clone'; + static safety = 'reference'; constructor(initialValue, { safety = Signal.safety, From 7388c0b6a89c7dd5afc34ad7628a179936d1936e Mon Sep 17 00:00:00 2001 From: Jack Date: Mon, 18 May 2026 17:43:16 -0400 Subject: [PATCH 03/19] Perf: Drop redundant createSnapshot in each-block reconcile The refChanged branch in each.js Phase 3 called refreshSnapshotAndDetect (which updates snapshot in place) and then unconditionally overwrote it with a fresh createSnapshot allocation. Reference mode hits this branch on position-change reconciles (e.g. filter cycling, list reordering); clone mode never enters it because identity churn routes through the sibling isArrayAsMode branch which doesn't have the double-snapshot. Match the sibling branch's pattern: only createSnapshot when no prior object snapshot existed. --- packages/renderer/src/engines/native/blocks/each.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/renderer/src/engines/native/blocks/each.js b/packages/renderer/src/engines/native/blocks/each.js index e0ac2702c..220e9ff48 100644 --- a/packages/renderer/src/engines/native/blocks/each.js +++ b/packages/renderer/src/engines/native/blocks/each.js @@ -416,9 +416,13 @@ function reconcile({ records, items, collectionType, node, data, scope, region, if (asValue !== null && typeof asValue === 'object') { let changedKeys = null; if (record.snapshot !== null && typeof record.snapshot === 'object') { + // refreshSnapshotAndDetect updates the snapshot in place — no need + // to re-allocate via createSnapshot afterwards. changedKeys = refreshSnapshotAndDetect(record.snapshot, asValue); } - record.snapshot = createSnapshot(asValue); + else { + record.snapshot = createSnapshot(asValue); + } if (changedKeys) { for (const key of changedKeys) { record.dataContext.notifyField(key); From 855e221e1c506f6f112560b9473aaacebca2f711 Mon Sep 17 00:00:00 2001 From: Jack Date: Mon, 18 May 2026 18:08:18 -0400 Subject: [PATCH 04/19] Bench: Move primitive-Signal benches to top of bench-signal.js MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fresh-take agents (Challenge + Survey + Neutral) all flagged that set-same-10m and sub-unsub-100k regressing under reference safety is inexplicable from the primitive-Signal hot path itself (protect() early-returns before reading this.safety; bytecode is identical to clone mode). The convergent hypothesis is that the regression is inherited cross-bench state: the upstream list-Signal benches do dramatically different allocation/cloning work in clone vs reference mode, leaving V8 with different JIT feedback / heap layout / GC pressure when the primitive benches execute later in the same Chrome session. Running the primitives first isolates them. If the regressions persist at the top of the script, the cause is intrinsic to the primitive path. If they disappear, the cause is cross-bench state — diagnostic, not a production code change. --- .../bench/tachometer/bench-signal.js | 76 ++++++++++--------- 1 file changed, 39 insertions(+), 37 deletions(-) diff --git a/packages/reactivity/bench/tachometer/bench-signal.js b/packages/reactivity/bench/tachometer/bench-signal.js index 216e53097..852bb0d75 100644 --- a/packages/reactivity/bench/tachometer/bench-signal.js +++ b/packages/reactivity/bench/tachometer/bench-signal.js @@ -21,6 +21,45 @@ const makeRecords = (n) => { let sink = null; +/******************************* + Primitive Signal hot paths + (placed first to isolate from + cross-bench heap/JIT state) +*******************************/ + +// set-same-10m — exercises the equality short-circuit. With no +// subscribers attached, set(same) collapses to an equality check + early +// return. V8 JIT inlines aggressively here (each set is ~8ns), so 10M +// iterations are needed to land above the σ-floor. A regression that +// bypasses the short-circuit (e.g., always-notify) inflates the per-set +// cost an order of magnitude and lights up immediately. +{ + const sig = new Signal(42); + // purpose: Sets a signal to its current value 10000000 times. Exercises the no-op fast path when nothing changes. + performance.mark(startMark('set-same-10m')); + for (let i = 0; i < 10_000_000; i++) { + sig.set(42); + } + performance.measure('set-same-10m', startMark('set-same-10m')); +} + +// sub-unsub-100k — measures the per-create/per-destroy cost of a +// subscriber that reads one signal. Components with frequent mount/unmount +// (modal dialogs, list virtualization, route transitions) hit this path +// continuously. 100k cycles to clear the σ-floor at ~340ns/cycle. +{ + const sig = new Signal(0); + // purpose: Creates and tears down a subscriber on one signal across 100000 cycles. Subscription churn cost. + performance.mark(startMark('sub-unsub-100k')); + for (let i = 0; i < 100_000; i++) { + const r = Reaction.create(() => { + sink = sig.get(); + }); + r.stop(); + } + performance.measure('sub-unsub-100k', startMark('sub-unsub-100k')); +} + /******************************* Reactive machinery *******************************/ @@ -223,43 +262,6 @@ let sink = null; r.stop(); } -/******************************* - Signal hot paths -*******************************/ - -// set-same-10m — exercises the equality short-circuit. With no -// subscribers attached, set(same) collapses to an equality check + early -// return. V8 JIT inlines aggressively here (each set is ~8ns), so 10M -// iterations are needed to land above the σ-floor. A regression that -// bypasses the short-circuit (e.g., always-notify) inflates the per-set -// cost an order of magnitude and lights up immediately. -{ - const sig = new Signal(42); - // purpose: Sets a signal to its current value 10000000 times. Exercises the no-op fast path when nothing changes. - performance.mark(startMark('set-same-10m')); - for (let i = 0; i < 10_000_000; i++) { - sig.set(42); - } - performance.measure('set-same-10m', startMark('set-same-10m')); -} - -// sub-unsub-100k — measures the per-create/per-destroy cost of a -// subscriber that reads one signal. Components with frequent mount/unmount -// (modal dialogs, list virtualization, route transitions) hit this path -// continuously. 100k cycles to clear the σ-floor at ~340ns/cycle. -{ - const sig = new Signal(0); - // purpose: Creates and tears down a subscriber on one signal across 100000 cycles. Subscription churn cost. - performance.mark(startMark('sub-unsub-100k')); - for (let i = 0; i < 100_000; i++) { - const r = Reaction.create(() => { - sink = sig.get(); - }); - r.stop(); - } - performance.measure('sub-unsub-100k', startMark('sub-unsub-100k')); -} - /******************************* Reaction scheduler *******************************/ From f36d04333d595e1d1a346344b2a4a7ff85d13811 Mon Sep 17 00:00:00 2001 From: Jack Date: Mon, 18 May 2026 18:10:42 -0400 Subject: [PATCH 05/19] Revert "Bench: Move primitive-Signal benches to top of bench-signal.js" This reverts commit 855e221e1c506f6f112560b9473aaacebca2f711. --- .../bench/tachometer/bench-signal.js | 76 +++++++++---------- 1 file changed, 37 insertions(+), 39 deletions(-) diff --git a/packages/reactivity/bench/tachometer/bench-signal.js b/packages/reactivity/bench/tachometer/bench-signal.js index 852bb0d75..216e53097 100644 --- a/packages/reactivity/bench/tachometer/bench-signal.js +++ b/packages/reactivity/bench/tachometer/bench-signal.js @@ -21,45 +21,6 @@ const makeRecords = (n) => { let sink = null; -/******************************* - Primitive Signal hot paths - (placed first to isolate from - cross-bench heap/JIT state) -*******************************/ - -// set-same-10m — exercises the equality short-circuit. With no -// subscribers attached, set(same) collapses to an equality check + early -// return. V8 JIT inlines aggressively here (each set is ~8ns), so 10M -// iterations are needed to land above the σ-floor. A regression that -// bypasses the short-circuit (e.g., always-notify) inflates the per-set -// cost an order of magnitude and lights up immediately. -{ - const sig = new Signal(42); - // purpose: Sets a signal to its current value 10000000 times. Exercises the no-op fast path when nothing changes. - performance.mark(startMark('set-same-10m')); - for (let i = 0; i < 10_000_000; i++) { - sig.set(42); - } - performance.measure('set-same-10m', startMark('set-same-10m')); -} - -// sub-unsub-100k — measures the per-create/per-destroy cost of a -// subscriber that reads one signal. Components with frequent mount/unmount -// (modal dialogs, list virtualization, route transitions) hit this path -// continuously. 100k cycles to clear the σ-floor at ~340ns/cycle. -{ - const sig = new Signal(0); - // purpose: Creates and tears down a subscriber on one signal across 100000 cycles. Subscription churn cost. - performance.mark(startMark('sub-unsub-100k')); - for (let i = 0; i < 100_000; i++) { - const r = Reaction.create(() => { - sink = sig.get(); - }); - r.stop(); - } - performance.measure('sub-unsub-100k', startMark('sub-unsub-100k')); -} - /******************************* Reactive machinery *******************************/ @@ -262,6 +223,43 @@ let sink = null; r.stop(); } +/******************************* + Signal hot paths +*******************************/ + +// set-same-10m — exercises the equality short-circuit. With no +// subscribers attached, set(same) collapses to an equality check + early +// return. V8 JIT inlines aggressively here (each set is ~8ns), so 10M +// iterations are needed to land above the σ-floor. A regression that +// bypasses the short-circuit (e.g., always-notify) inflates the per-set +// cost an order of magnitude and lights up immediately. +{ + const sig = new Signal(42); + // purpose: Sets a signal to its current value 10000000 times. Exercises the no-op fast path when nothing changes. + performance.mark(startMark('set-same-10m')); + for (let i = 0; i < 10_000_000; i++) { + sig.set(42); + } + performance.measure('set-same-10m', startMark('set-same-10m')); +} + +// sub-unsub-100k — measures the per-create/per-destroy cost of a +// subscriber that reads one signal. Components with frequent mount/unmount +// (modal dialogs, list virtualization, route transitions) hit this path +// continuously. 100k cycles to clear the σ-floor at ~340ns/cycle. +{ + const sig = new Signal(0); + // purpose: Creates and tears down a subscriber on one signal across 100000 cycles. Subscription churn cost. + performance.mark(startMark('sub-unsub-100k')); + for (let i = 0; i < 100_000; i++) { + const r = Reaction.create(() => { + sink = sig.get(); + }); + r.stop(); + } + performance.measure('sub-unsub-100k', startMark('sub-unsub-100k')); +} + /******************************* Reaction scheduler *******************************/ From f4da3d76f1fc140748919e482da08bd7ef3a980d Mon Sep 17 00:00:00 2001 From: Jack Date: Mon, 18 May 2026 19:37:34 -0400 Subject: [PATCH 06/19] Harness: Add investigate-performance contributing skill MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Captures the flow that produced real results in the signal-safety investigation: chrome-devtools MCP traces, counter instrumentation of bundled framework, local tachometer with custom-built bundles, fresh-take subagents when reasoning loops. Documents the dead ends that didn't work — static reading, V8 hypothesis without skill citation, pushing bench-file changes that get overlaid away. --- .../contributing/investigate-performance.md | 336 ++++++++++++++++++ 1 file changed, 336 insertions(+) create mode 100644 ai/skills/contributing/investigate-performance.md diff --git a/ai/skills/contributing/investigate-performance.md b/ai/skills/contributing/investigate-performance.md new file mode 100644 index 000000000..4a66bb43c --- /dev/null +++ b/ai/skills/contributing/investigate-performance.md @@ -0,0 +1,336 @@ +--- +title: Investigating Performance Regressions +description: The flow that produces real diagnoses for perf regressions in this codebase — from CI bench reports to Chrome DevTools traces to counter instrumentation. Covers the dead ends to avoid (static reading, hidden-class hypotheses without verification, bench-file edits that get overlaid away) and the techniques that actually localize root cause. +keywords: [performance investigation, perf regression, chrome devtools, MCP profile, counter instrumentation, tachometer local, fresh-take, V8 profiling, bench harness, reactive call count] +audience: contributing +skill: investigate-performance +type: skill +--- + +# Investigating Performance Regressions + +> **Skill:** `investigate-performance` +> **Purpose:** The flow that actually produces root cause for perf regressions on this codebase. Static reading and theorizing fail repeatedly. Use this as the order of operations. + +**Golden rule: profile before you theorize.** A Chrome DevTools trace + a few injected counters localize the mechanism in minutes. Reading framework source files trying to derive why something is slow burns hours and converges on plausible-sounding wrong answers. Start with measurement. + +--- + +## When to Reach for This Skill + +The bench bot reports a confident regression on a PR (`/read-bench-report` shows it in the ❌ Slower bucket). You've ruled out noise (CIs are tight, multiple runs reproduce). The CI bench delta is real — now you need to know *why*. + +This skill is for that *why* phase, not the *did it regress* phase. + +--- + +## The Flow That Works + +``` +1. Identify the bench targets (weighted) → /read-bench-report +2. Read the relevant V8 skills FIRST → ground claims against current engine behavior +3. Capture Chrome traces, ref vs main, of one bench → chrome-devtools MCP +4. Diff hot functions (inclusive + exclusive self-time) → identify suspects +5. Inject counters into the bundle → tell more-calls vs slower-per-call +6. Repeat with a tighter hypothesis → bisect into the framework path +7. Spawn /fresh-take subagents if static reasoning loops → escape solution momentum +``` + +Steps 3-5 are the breakthrough. Each step is cheap (~5-10 minutes) and produces empirical signal. + +--- + +## Step 1 — Identify Targets With Weights + +Not all regressions are equal. The PR author or maintainer has weights. Typical pattern in this codebase: + +| package suite | weight | rationale | +|---|---:|---| +| `krausest` | 5× | js-framework-benchmark, headline external comparison | +| `todo`, `template`, `hydrate` | 1× | end-user workloads | +| `signal`, `compiler-micros`, `renderer-micros` | 0.25× | internal microbenches, useful but not user-facing | + +Build a target table: bench name, weight, **ref-vs-clone (or PR-vs-main) delta in percentage points**. Order by weight × magnitude. Investigate the heaviest first; stop when remaining gaps are noise-floor adjacent (~±4% on short benches). + +**Do not** burn cycles on borderline-noise regressions — `/read-bench-report` documents the noise-floor envelope per duration. + +--- + +## Step 2 — Ground Claims Against Current V8 Behavior + +Before reasoning about *why* a regression exists, read these: + +- `performance-v8-overview` — tier model context +- `performance-v8-object-model` — hidden classes, IC monomorphic/poly/megamorphic, allocation site dedup (this skill specifically debunks "objects from different source locations are different shapes", which is a common wrong intuition) +- `performance-v8-memory` — GC generations, young-gen is cheap, Proxy is not specialized +- `performance-v8-stale-advice` — the firewall against pre-2022 V8 folklore (object.freeze speed, try/catch deopts, blanket monomorphism cargo culting) +- `performance-v8-compilation` — feedback stability, deopt triggers, what Maglev/Turbofan inline + +These exist precisely because agents bring incorrect priors from training data. **Cite them when making claims.** When you find yourself reasoning about V8 IC behavior without a citation, stop and verify. + +A common failure mode: an agent forms a hidden-class polymorphism hypothesis, then proceeds for ~30 minutes building elaborate theory around it. The object-model skill explicitly says "two `{a:1, b:2}` literals at different lines share a shape." Reading that one paragraph would have prevented the entire detour. + +--- + +## Step 3 — Capture Chrome DevTools Traces + +The chrome-devtools MCP server lets you capture a full V8 sampling trace from inside the conversation. This is the single most informative tool in this flow. + +**Setup:** + +1. Build the bench bundles you want to compare (e.g. `current` = ref-mode signal.js, `baseline` = clone-mode signal.js): + + ```bash + cd packages//bench/tachometer + node build-ci.js current + # swap the safety flag, rebuild baseline + sed -i "s/static safety = 'reference';/static safety = 'clone';/" \ + /home/jack/semantic/next/packages/reactivity/src/signal.js + node build-ci.js baseline + sed -i "s/static safety = 'clone';/static safety = 'reference';/" \ + /home/jack/semantic/next/packages/reactivity/src/signal.js + ``` + +2. Serve the repo over HTTP from a stable port: + + ```bash + python3 -m http.server 8766 --directory /home/jack/semantic/next & + ``` + +3. Navigate the MCP browser to the bench page, capture the trace: + + ``` + mcp__chrome-devtools__navigate_page url=...ci-current-todo.html + mcp__chrome-devtools__performance_start_trace autoStop=false filePath=/tmp/trace-ref.json + # poll for performance.measure('rename-500') or similar end-marker + mcp__chrome-devtools__performance_stop_trace filePath=/tmp/trace-ref.json + ``` + +4. Repeat for the other variant (`ci-baseline-todo.html` → `/tmp/trace-clone.json`). + +### Diff hot functions + +A traced file is ~150-200MB JSON. Parse it offline and aggregate self-time per function within the target bench's `performance.measure` region: + +```js +// /tmp/parse.mjs +import fs from 'node:fs'; +const t = JSON.parse(fs.readFileSync('/tmp/trace-ref.json', 'utf8')); +const events = t.traceEvents; +// 1) find region: events.filter(e => e.name === 'toggle-all-200' && e.cat?.includes('blink.user_timing')) +// use ph='b' (begin) and ph='e' (end) timestamps +// 2) Build profile node table from ProfileChunk events; build sample stream with cumulative timestamps +// 3) Filter samples within [regionStart, regionEnd] +// 4) Aggregate selfTime by node.callFrame.functionName + url:line +// (normalize 'current/' vs 'baseline/' bundle paths) +// 5) Same for inclusive — walk each sample's stack via parent map built from node.children +// 6) Diff ref-self vs clone-self per function — biggest positive Δ is your suspect +``` + +The full parsing script used in the signal-safety investigation is at `ai/workspace/artifacts/signal-safety-regressions/` if you need a starting point. + +**What "hot" looks like.** A regression where 85% of the gap concentrates in 1-2 functions is much easier to diagnose than one spread across 20. If you see the latter, the mechanism is probably structural (allocation pattern, GC, scheduler) rather than a single hot loop. + +### Single-capture limitations + +Single Chrome traces are not statistically rigorous — variance is high. But for *identifying which function the cost lives in*, one capture is usually enough. You're not measuring the delta — CI bench already did that — you're localizing where the time is spent. + +--- + +## Step 4 — Inject Counters to Distinguish More-Calls vs Slower-Per-Call + +The trace gives self-time per function. It does NOT give call count. Without call count, you can't tell: + +- "this function is intrinsically slower per call in mode A" (deopt, IC pollution, allocator pressure) +- "this function is called more times in mode A" (extra reactive wakeups, cascade in a hot loop) + +These have different fixes. Distinguish them with injected counters. + +### The pattern + +Patch the bundle file *after build*, between `build-ci.js` and serving: + +```bash +DIR=packages//bench/tachometer +for variant in current baseline; do + FILE=$DIR/dist/$variant/bench-todo.js + # Find the function in the bundle, inject a counter on first line + sed -i 's|evaluateJavascript(code, context, { includeHelpers = true } = {}) {|evaluateJavascript(code, context, { includeHelpers = true } = {}) {\n if (typeof window !== "undefined" \&\& window.__counters) window.__counters.evals++;|' $FILE + # Same pattern for notifyField, Dependency.changed, Reaction.run, etc. +done +``` + +The counters are global on `window.__counters`. Initialize them via `initScript` on `mcp__chrome-devtools__navigate_page`: + +```js +// initScript runs before any bundle code +window.__counters = { evals: 0, notifies: 0, depFires: 0 }; +window.__byMeasure = {}; +const _mark = performance.mark.bind(performance); +const _measure = performance.measure.bind(performance); +// Wrap mark/measure to snapshot counter values per region +performance.mark = function(name, opts) { + if (typeof name === 'string' && name.endsWith('-start')) { + const m = name.slice(0, -'-start'.length); + window.__byMeasure[m] = { _start: { ...window.__counters } }; + } + return _mark(name, opts); +}; +performance.measure = function(name, start) { + const r = _measure(name, start); + const m = window.__byMeasure[name]; + if (m) { + const c = window.__counters, s = m._start; + Object.keys(c).forEach(k => { m[k] = c[k] - s[k]; }); + delete m._start; + } + return r; +}; +``` + +Navigate, wait for the bench to finish, read `window.__byMeasure`: + +```js +mcp__chrome-devtools__evaluate_script function=`async () => { + while (Date.now() < deadline) { + if (performance.getEntriesByType('measure').find(e => e.name === 'rename-500')) break; + await new Promise(r => setTimeout(r, 500)); + } + return { byMeasure: window.__byMeasure }; +}` +``` + +### What this distinguishes + +Run both modes. Compare `byMeasure['toggle-all-200']` across modes: + +- If `evals` count is equal and self-time differs → **per-call cost** is the issue (V8 specialization, IC behavior). Drill deeper into what changes in the per-call path. +- If `evals` count differs proportionally to the wall-clock delta → **more calls** is the issue. Find which call site is firing extra. + +In the signal-safety investigation, this is precisely what cracked it open: ref mode showed +20% eval count *and* +20% wall-clock on every regressing bench, with non-regressing benches showing equal counts. Locked-step 1.20× ratio told us the regression was extra reactive work, not slower per-operation. + +### Counter granularity hierarchy + +Start broad, narrow down: + +1. `evaluateJavascript` — counts JS-style expression evaluations +2. `notifyField` — counts per-field dep fires from each-block reconcile +3. `Dependency.changed` — counts all dep fires (broader than notifyField) +4. `Reaction.run` — counts Reaction re-runs (= binding wakes) +5. `Signal.set value` setter — counts every signal write + +Adding more counters per investigation iteration narrows the suspect down to a specific call site. + +--- + +## Step 5 — Local Tachometer for Hypothesis Tests + +When you want to test a hypothesis ("does reordering the bench eliminate the regression?", "does changing this expression form change the result?"), you can run tachometer locally with custom-built bundles. + +**Critical constraint that's easy to miss:** the CI bench workflow at `.github/workflows/benchmarks.yml:115-122` overlays `packages/*/bench/` from main before building either bundle. **This means PR-level changes to bench files are silently discarded by CI.** It's a deliberate anti-gaming defense. + +So: bench-file experiments must be tested LOCALLY, not via CI push. Or, if the change is genuinely a methodology improvement (e.g. fixing an `await rAF` antipattern), push it to main first as a `Bench:` commit, then re-run the PR's bench. + +**Local tachometer flow:** + +```bash +# Build both bundles from your working tree +cd packages//bench/tachometer +node build-ci.js current +# Modify source (e.g. swap signal.js mode, edit a bench file) +node build-ci.js baseline +# Restore source + +# Write tachometer config + HTML files pointing at the two bundle paths +# Then: +/home/jack/semantic/next/node_modules/.bin/tachometer \ + --config $(pwd)/my-experiment.json \ + --json-file /tmp/experiment.json +``` + +**Wrapper script pattern** — the harness resets cwd between commands, so wrap tachometer in a shell script: + +```bash +#!/bin/bash +cd /home/jack/semantic/next/packages//bench/tachometer || exit 1 +exec /home/jack/semantic/next/node_modules/.bin/tachometer \ + --config $(pwd)/my-experiment.json --json-file /tmp/out.json +``` + +Tachometer takes 3-7 minutes for ~30 samples × 2 configs. Output JSON has `benchmarks[].differences[]` with `percentChange.{low,high}` — that's your 95% CI for the experiment. + +See `/extend-bench-suite` for the full local-tachometer reference. + +--- + +## Step 6 — Read Bundle Source to Identify Suspect Code Paths + +Only AFTER you have an empirical signal (trace + counter data) is reading source code productive. The trace tells you *which function*; you read source to understand *what mechanism within that function* differs between variants. + +Cross-reference: + +- The bundle line numbers in the trace point you at the post-bundling line, not the source. Use `grep -n 'functionName' dist/current/bench-*.js` to find context. +- Map back to source: `grep -rn 'functionName' packages/*/src/`. +- For framework primitives that the bench uses, the relevant skills are in `ai/skills/contributing/`: + - `native-renderer` for the each-block reconcile, per-item Proxy, scope.reaction binding + - `internals` for higher-level framework architecture + +--- + +## Step 7 — Spawn Fresh-Take Agents When Reasoning Loops + +When you've been on the same hypothesis for an hour and the data isn't agreeing, you're probably in the failure mode the `fresh-take` skill targets: solution momentum. + +Per `/fresh-take`: + +1. Write a brief that captures **problem knowledge** (observations, constraints, V8 skill references) **but not solution momentum** (your specific hypotheses, code paths you've explored). +2. Save to `ai/workspace/artifacts//-evaluation.md`. +3. Spawn 2-3 subagents with different lenses (neutral, challenge, survey). +4. Audit each subagent's output for V8 skill citations — if they didn't cite `performance-v8-object-model` or `performance-v8-stale-advice`, their priors may be uncalibrated. +5. Present findings to user without reconciliation, per the skill. + +This worked in the signal-safety investigation: the Challenge and Survey agents independently identified that *more reactive work* (not slower per-call) was the likely mechanism, which the counter instrumentation then confirmed. + +--- + +## Dead Ends to Avoid + +**Static code reading without measurement.** Convinces you of a mechanism, then the data disagrees. The trace tells you in 5 minutes what reading takes hours to maybe-guess. + +**Speculating about V8 specialization without citation.** V8 has moved a lot since 2017. If you're claiming "X is slow because of polymorphism" or "Y causes deopt", cite the specific V8 skill paragraph or v8.dev post. Otherwise you're probably wrong. + +**Pushing bench-file changes to CI to test hypotheses.** The benchmarks workflow overlays `packages/*/bench/` from main before building. Your changes are silently discarded. Test locally. + +**Mistaking allocation cost for the dominant cost.** Per `performance-v8-memory`, young-gen allocation is cheap. "Clone mode allocates more therefore clone mode is slower" is wrong on its own — surviving into old gen is the expensive part. Verify with actual GC profiling, not allocation counting. + +**Equating Chrome trace self-time with call count.** Self-time is sample-based and reflects time spent. Call count requires instrumentation. The two answer different questions. + +**Trying to make the regression disappear by changing the bench code.** The bench is there to expose the regression. Routing around it (e.g. rewriting a JS-eval expression to Lisp form) hides the symptom; it doesn't diagnose the cause. The maintainer rightly pushes back on this. + +--- + +## Quick Reference + +| Phase | Tool | Output | +|---|---|---| +| Targets | `/read-bench-report` | weighted bench list | +| Ground claims | `performance-v8-*` skills | V8 priors are calibrated | +| Profile | chrome-devtools MCP `performance_start_trace` | per-function self-time | +| Distinguish call-count vs per-call | sed-injected `window.__counters` | per-region call counts | +| Hypothesis test | local tachometer with custom bundles | ref-vs-X percentChange CI | +| Stuck | `/fresh-take` with workspace brief | independent subagent reads | + +--- + +## Related Skills + +| Skill | Command | Use when... | +|-------|---------|-------------| +| **Reading Bench Reports** | `/read-bench-report` | Interpreting the bench-bot PR comment | +| **Extend Bench Suite** | `/extend-bench-suite` | Adding a new bench, local tachometer setup | +| **Improve Performance** | `/improve-performance` | Audit-fix-measure cycle for an entire package | +| **Fresh Take** | `/fresh-take` | Escaping solution momentum with subagent delegation | +| **V8 Object Model** | `performance-v8-object-model` | Hidden classes, IC, polymorphism reality check | +| **V8 Memory** | `performance-v8-memory` | GC, allocation cost, Proxy speed | +| **V8 Stale Advice** | `performance-v8-stale-advice` | Firewall against pre-2022 perf folklore | +| **Native Renderer** | `native-renderer` | Each-block reconcile, per-item Proxy, binding dispatch | From 4b162d6b5139d2f0c5c8d54fbea36272fd00b910 Mon Sep 17 00:00:00 2001 From: jlukic Date: Tue, 19 May 2026 23:37:09 -0400 Subject: [PATCH 07/19] Test: Fix tests with new expects from new signal safety shape --- .../reactivity/test/unit/internals.test.js | 10 +++- packages/reactivity/test/unit/signal.test.js | 21 +++++--- .../test/browser/ssr-hydration.test.js | 53 +++++++++++++++++-- 3 files changed, 74 insertions(+), 10 deletions(-) diff --git a/packages/reactivity/test/unit/internals.test.js b/packages/reactivity/test/unit/internals.test.js index 08b0b19b8..86ab6dc33 100644 --- a/packages/reactivity/test/unit/internals.test.js +++ b/packages/reactivity/test/unit/internals.test.js @@ -785,9 +785,17 @@ describe('Signal — read paths via internals', () => { expect(lastSeen.a).toBe(1); }); - it('peek returns a clone (defensive read) but no dependency', () => { + it('peek returns the live reference by default (reference safety)', () => { const s = new Signal([1, 2, 3]); const peeked = s.peek(); + expect(peeked).toBe(s.raw()); + peeked.push(99); + expect(s.peek()).toEqual([1, 2, 3, 99]); + }); + + it('peek returns a defensive clone under clone safety', () => { + const s = new Signal([1, 2, 3], { safety: 'clone' }); + const peeked = s.peek(); peeked.push(99); expect(s.peek()).toEqual([1, 2, 3]); }); diff --git a/packages/reactivity/test/unit/signal.test.js b/packages/reactivity/test/unit/signal.test.js index 7d06f26e6..ac68a1deb 100644 --- a/packages/reactivity/test/unit/signal.test.js +++ b/packages/reactivity/test/unit/signal.test.js @@ -701,11 +701,20 @@ describe('Signal API', () => { expect(empty.get()).toBe(null); }); - it('clones object initial values by default so external mutations do not leak in', () => { + it('does not clone object initial values by default (reference safety shares the reference)', () => { const original = { count: 0 }; const signal = new Signal(original); original.count = 99; - // Internal state should be insulated from later mutation of the source + // Reference safety is the default — the signal shares the reference, + // so later mutation of the source is visible through it. + expect(signal.peek().count).toBe(99); + }); + + it('clones object initial values under clone safety so external mutations do not leak in', () => { + const original = { count: 0 }; + const signal = new Signal(original, { safety: 'clone' }); + original.count = 99; + // Clone safety insulates internal state from later source mutation expect(signal.peek().count).toBe(0); }); @@ -738,9 +747,9 @@ describe('Signal API', () => { expect(callback).toHaveBeenCalledTimes(2); }); - it('uses a custom clone function when provided', () => { + it('uses a custom clone function when provided under clone safety', () => { const cloneSpy = vi.fn(value => ({ ...value, cloned: true })); - const signal = new Signal({ name: 'Alice' }, { cloneFunction: cloneSpy }); + const signal = new Signal({ name: 'Alice' }, { safety: 'clone', cloneFunction: cloneSpy }); expect(cloneSpy).toHaveBeenCalled(); expect(signal.peek().cloned).toBe(true); }); @@ -845,8 +854,8 @@ describe('Signal API', () => { expect(cb).toHaveBeenCalledTimes(2); }); - it('clones object values just like get(), insulating callers from mutation', () => { - const signal = new Signal({ level: 1 }); + it('clones object values just like get() under clone safety, insulating callers from mutation', () => { + const signal = new Signal({ level: 1 }, { safety: 'clone' }); const snapshot = signal.peek(); snapshot.level = 99; expect(signal.peek().level).toBe(1); diff --git a/packages/renderer/test/browser/ssr-hydration.test.js b/packages/renderer/test/browser/ssr-hydration.test.js index cb5a3f9d3..ed97b53d1 100644 --- a/packages/renderer/test/browser/ssr-hydration.test.js +++ b/packages/renderer/test/browser/ssr-hydration.test.js @@ -1028,6 +1028,51 @@ describe('SSR hydration — post-hydration list mutations', () => { expect(el.shadowRoot.querySelectorAll('li')[2].textContent).toBe('c'); }); + it('does not spuriously re-evaluate unchanged item bindings after hydration', async () => { + // The fix (adopted records start fresh:false) lets the first reconcile + // run the per-field snapshot diff. This guards that the diff stays + // change-gated: an unrelated mutation must not wake the binding of an + // item whose fields are untouched. Counting helper evaluations catches + // a binding-level spurious wake that node-identity checks would miss. + let nameFires = 0; + const el = await ssrAndHydrate({ + template: '{#each item in items}
  • {markName item.name}
    • {/each}', + defaultState: { items: [{ name: 'Red' }, { name: 'Blue' }] }, + createComponent: ({ state }) => ({ + markName: (name) => { + nameFires++; + return name; + }, + renameFirst() { + state.items.setArrayProperty(0, 'name', 'Crimson'); + }, + appendItem() { + state.items.push({ name: 'Green' }); + }, + }), + }); + + expect([...el.shadowRoot.querySelectorAll('li.r')].map(n => n.textContent)).toEqual(['Red', 'Blue']); + + // Append a 3rd item: items 0 and 1 are unchanged. Exactly one new binding + // evaluation (the appended item) — the hydrated, server-valid items must + // not re-evaluate. + const baseline = nameFires; + const updated1 = $(el).onNext('updated'); + el.component.appendItem(); + await updated1; + expect([...el.shadowRoot.querySelectorAll('li.r')].map(n => n.textContent)).toEqual(['Red', 'Blue', 'Green']); + expect(nameFires - baseline).toBe(1); + + // Mutate only item 0's field: exactly one re-evaluation (item 0), not 1 or 2. + const afterAppend = nameFires; + const updated2 = $(el).onNext('updated'); + el.component.renameFirst(); + await updated2; + expect([...el.shadowRoot.querySelectorAll('li.r')].map(n => n.textContent)).toEqual(['Crimson', 'Blue', 'Green']); + expect(nameFires - afterAppend).toBe(1); + }); + it('each list shrinks after hydration', async () => { const el = await ssrAndHydrate({ template: '{#each item in getItems}{item}{/each}', @@ -1479,9 +1524,11 @@ describe('SSR hydration — snippet reactivity', () => { defaultState: { items: [{ name: 'Red' }, { name: 'Blue' }] }, createComponent: ({ state }) => ({ renameFirst() { - const items = state.items.peek(); - items[0] = { ...items[0], name: 'Crimson' }; - state.items.set([...items]); + // In-place field mutation: keeps item identity (refChanged=false), + // exercising the same-ref snapshot-diff reconcile path. Under + // reference safety this is the canonical mutation; get-mutate-set + // through peek() would corrupt set()'s dedup baseline and no-op. + state.items.setArrayProperty(0, 'name', 'Crimson'); }, }), }); From b6302491b3b7dc506004d5260615d89189f40683 Mon Sep 17 00:00:00 2001 From: jlukic Date: Tue, 19 May 2026 23:41:13 -0400 Subject: [PATCH 08/19] Bug: Fix each bug from signal pass by ref on hydration --- packages/renderer/src/engines/native/blocks/each.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/renderer/src/engines/native/blocks/each.js b/packages/renderer/src/engines/native/blocks/each.js index 220e9ff48..65fa55d2a 100644 --- a/packages/renderer/src/engines/native/blocks/each.js +++ b/packages/renderer/src/engines/native/blocks/each.js @@ -613,7 +613,12 @@ function adoptServerItems({ scope: itemScope, isElse: false, snapshot: createSnapshot(item), - fresh: true, + // Not fresh: unlike a record created during reconcile, an adopted + // record's bindings were wired during hydration, so it already + // carries subscribers that a first in-place field mutation must + // wake. fresh:true would gate it out of the same-ref snapshot-diff + // branch and drop that mutation. + fresh: false, }); } else { From c6deb69fdbab6c4704f8843db397534c06ab166c Mon Sep 17 00:00:00 2001 From: jlukic Date: Tue, 19 May 2026 23:49:16 -0400 Subject: [PATCH 09/19] Harness: Update investigate performance skill --- .../contributing/investigate-performance.md | 45 +++++++++---------- 1 file changed, 20 insertions(+), 25 deletions(-) diff --git a/ai/skills/contributing/investigate-performance.md b/ai/skills/contributing/investigate-performance.md index 4a66bb43c..f4a571059 100644 --- a/ai/skills/contributing/investigate-performance.md +++ b/ai/skills/contributing/investigate-performance.md @@ -29,7 +29,7 @@ This skill is for that *why* phase, not the *did it regress* phase. ``` 1. Identify the bench targets (weighted) → /read-bench-report 2. Read the relevant V8 skills FIRST → ground claims against current engine behavior -3. Capture Chrome traces, ref vs main, of one bench → chrome-devtools MCP +3. Capture Chrome traces, PR vs main, of one bench → chrome-devtools MCP 4. Diff hot functions (inclusive + exclusive self-time) → identify suspects 5. Inject counters into the bundle → tell more-calls vs slower-per-call 6. Repeat with a tighter hypothesis → bisect into the framework path @@ -50,7 +50,7 @@ Not all regressions are equal. The PR author or maintainer has weights. Typical | `todo`, `template`, `hydrate` | 1× | end-user workloads | | `signal`, `compiler-micros`, `renderer-micros` | 0.25× | internal microbenches, useful but not user-facing | -Build a target table: bench name, weight, **ref-vs-clone (or PR-vs-main) delta in percentage points**. Order by weight × magnitude. Investigate the heaviest first; stop when remaining gaps are noise-floor adjacent (~±4% on short benches). +Build a target table: bench name, weight, **PR-vs-main delta in percentage points**. Order by weight × magnitude. Investigate the heaviest first; stop when remaining gaps are noise-floor adjacent (~±4% on short benches). **Do not** burn cycles on borderline-noise regressions — `/read-bench-report` documents the noise-floor envelope per duration. @@ -78,23 +78,21 @@ The chrome-devtools MCP server lets you capture a full V8 sampling trace from in **Setup:** -1. Build the bench bundles you want to compare (e.g. `current` = ref-mode signal.js, `baseline` = clone-mode signal.js): +1. Build the bench bundles you want to compare. `current` is your working tree (the PR change), `baseline` is the same tree with the change reverted. The pattern is: build one, flip the source line under test, build the other, restore: ```bash cd packages//bench/tachometer node build-ci.js current - # swap the safety flag, rebuild baseline - sed -i "s/static safety = 'reference';/static safety = 'clone';/" \ - /home/jack/semantic/next/packages/reactivity/src/signal.js + # flip the single source line under test, rebuild baseline, then restore it node build-ci.js baseline - sed -i "s/static safety = 'clone';/static safety = 'reference';/" \ - /home/jack/semantic/next/packages/reactivity/src/signal.js ``` + Keep the flip to the smallest possible diff so the two bundles isolate exactly the change you're investigating. + 2. Serve the repo over HTTP from a stable port: ```bash - python3 -m http.server 8766 --directory /home/jack/semantic/next & + python3 -m http.server 8766 --directory & ``` 3. Navigate the MCP browser to the bench page, capture the trace: @@ -124,11 +122,9 @@ const events = t.traceEvents; // 4) Aggregate selfTime by node.callFrame.functionName + url:line // (normalize 'current/' vs 'baseline/' bundle paths) // 5) Same for inclusive — walk each sample's stack via parent map built from node.children -// 6) Diff ref-self vs clone-self per function — biggest positive Δ is your suspect +// 6) Diff current-self vs baseline-self per function — biggest positive Δ is your suspect ``` -The full parsing script used in the signal-safety investigation is at `ai/workspace/artifacts/signal-safety-regressions/` if you need a starting point. - **What "hot" looks like.** A regression where 85% of the gap concentrates in 1-2 functions is much easier to diagnose than one spread across 20. If you see the latter, the mechanism is probably structural (allocation pattern, GC, scheduler) rather than a single hot loop. ### Single-capture limitations @@ -207,7 +203,7 @@ Run both modes. Compare `byMeasure['toggle-all-200']` across modes: - If `evals` count is equal and self-time differs → **per-call cost** is the issue (V8 specialization, IC behavior). Drill deeper into what changes in the per-call path. - If `evals` count differs proportionally to the wall-clock delta → **more calls** is the issue. Find which call site is firing extra. -In the signal-safety investigation, this is precisely what cracked it open: ref mode showed +20% eval count *and* +20% wall-clock on every regressing bench, with non-regressing benches showing equal counts. Locked-step 1.20× ratio told us the regression was extra reactive work, not slower per-operation. +A locked-step ratio is the strongest signal here: when the call-count delta and the wall-clock delta move by the same factor across every regressing bench (and non-regressing benches show equal counts), the regression is extra work being scheduled, not slower per-operation. That distinction points at completely different fixes, so it's worth confirming before you touch source. ### Counter granularity hierarchy @@ -237,13 +233,13 @@ So: bench-file experiments must be tested LOCALLY, not via CI push. Or, if the c # Build both bundles from your working tree cd packages//bench/tachometer node build-ci.js current -# Modify source (e.g. swap signal.js mode, edit a bench file) +# Modify source (e.g. flip the source line under test, edit a bench file) node build-ci.js baseline # Restore source # Write tachometer config + HTML files pointing at the two bundle paths # Then: -/home/jack/semantic/next/node_modules/.bin/tachometer \ +/node_modules/.bin/tachometer \ --config $(pwd)/my-experiment.json \ --json-file /tmp/experiment.json ``` @@ -252,8 +248,8 @@ node build-ci.js baseline ```bash #!/bin/bash -cd /home/jack/semantic/next/packages//bench/tachometer || exit 1 -exec /home/jack/semantic/next/node_modules/.bin/tachometer \ +cd /packages//bench/tachometer || exit 1 +exec /node_modules/.bin/tachometer \ --config $(pwd)/my-experiment.json --json-file /tmp/out.json ``` @@ -284,12 +280,11 @@ When you've been on the same hypothesis for an hour and the data isn't agreeing, Per `/fresh-take`: 1. Write a brief that captures **problem knowledge** (observations, constraints, V8 skill references) **but not solution momentum** (your specific hypotheses, code paths you've explored). -2. Save to `ai/workspace/artifacts//-evaluation.md`. -3. Spawn 2-3 subagents with different lenses (neutral, challenge, survey). -4. Audit each subagent's output for V8 skill citations — if they didn't cite `performance-v8-object-model` or `performance-v8-stale-advice`, their priors may be uncalibrated. -5. Present findings to user without reconciliation, per the skill. +2. Spawn 2-3 subagents with different lenses (neutral, challenge, survey). +3. Audit each subagent's output for V8 skill citations — if they didn't cite `performance-v8-object-model` or `performance-v8-stale-advice`, their priors may be uncalibrated. +4. Present findings to user without reconciliation, per the skill. -This worked in the signal-safety investigation: the Challenge and Survey agents independently identified that *more reactive work* (not slower per-call) was the likely mechanism, which the counter instrumentation then confirmed. +The value here is that fresh agents, unanchored to the hypothesis you've been circling, often name the mechanism you'd ruled out prematurely. Counter instrumentation can then confirm or kill their read empirically. --- @@ -301,7 +296,7 @@ This worked in the signal-safety investigation: the Challenge and Survey agents **Pushing bench-file changes to CI to test hypotheses.** The benchmarks workflow overlays `packages/*/bench/` from main before building. Your changes are silently discarded. Test locally. -**Mistaking allocation cost for the dominant cost.** Per `performance-v8-memory`, young-gen allocation is cheap. "Clone mode allocates more therefore clone mode is slower" is wrong on its own — surviving into old gen is the expensive part. Verify with actual GC profiling, not allocation counting. +**Mistaking allocation cost for the dominant cost.** Per `performance-v8-memory`, young-gen allocation is cheap. "This variant allocates more therefore it's slower" is wrong on its own — surviving into old gen is the expensive part. Verify with actual GC profiling, not allocation counting. **Equating Chrome trace self-time with call count.** Self-time is sample-based and reflects time spent. Call count requires instrumentation. The two answer different questions. @@ -317,8 +312,8 @@ This worked in the signal-safety investigation: the Challenge and Survey agents | Ground claims | `performance-v8-*` skills | V8 priors are calibrated | | Profile | chrome-devtools MCP `performance_start_trace` | per-function self-time | | Distinguish call-count vs per-call | sed-injected `window.__counters` | per-region call counts | -| Hypothesis test | local tachometer with custom bundles | ref-vs-X percentChange CI | -| Stuck | `/fresh-take` with workspace brief | independent subagent reads | +| Hypothesis test | local tachometer with custom bundles | PR-vs-variant percentChange CI | +| Stuck | `/fresh-take` with problem-knowledge brief | independent subagent reads | --- From 37dd5ada4d999a0034aed8ccc3ece10bb5a8c176 Mon Sep 17 00:00:00 2001 From: jlukic Date: Wed, 20 May 2026 08:44:35 -0400 Subject: [PATCH 10/19] Harness: Rework investigate-performance for weighting, orientation, and evidence integrity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a fixed bench-weight heuristic (krausest 5x / todo·template·hydrate 2x / synthetics 0.25x) with a gate that the heaviest real-workload regressor must be measured, not reasoned about by analogy. Splits the instrument flow into gather (trace, baseline diff — no hypothesis) and steelman (counters/Playwright — hypothesis required), with the hypothesis born from a measurement rather than a read. Adds an orientation step: read the bench and learn how the component works user-side via the authoring curriculum, since the renderer is a separate package from the component surface. Adds an evidence-integrity rule for handling prior cause-claims, and keeps the no-bench-editing guardrail. --- .../contributing/investigate-performance.md | 107 +++++++++++++----- 1 file changed, 80 insertions(+), 27 deletions(-) diff --git a/ai/skills/contributing/investigate-performance.md b/ai/skills/contributing/investigate-performance.md index f4a571059..44b15bf81 100644 --- a/ai/skills/contributing/investigate-performance.md +++ b/ai/skills/contributing/investigate-performance.md @@ -14,6 +14,24 @@ type: skill **Golden rule: profile before you theorize.** A Chrome DevTools trace + a few injected counters localize the mechanism in minutes. Reading framework source files trying to derive why something is slow burns hours and converges on plausible-sounding wrong answers. Start with measurement. +**A conclusion is the measurement chain you produced, not the destination.** Naming a cause is worth nothing without a chain of measurements *you ran in this investigation* that forces it. Two things follow: + +- **Investigate the regression a user feels, not the one that's easy to isolate.** A synthetic microbench getting slower (setting a signal to itself, subscribe/unsubscribe churn) is rarely worth acting on. A real component workload getting slower (editing a todo, rendering a list) is the finding. The synthetic bench is tempting because it isolates cleanly — the weight gate in Step 1 keeps you honest about which one to chase. +- **A prior cause-claim is a hypothesis, not an answer.** The cause may already be written down — a commit message, a PR comment, an `ai/workspace` note, a code comment. Treat it as a lead to test, never as evidence. See "Evidence Integrity" below. + +--- + +## Evidence Integrity — prior claims are leads, not answers + +You will read prior explanations of the regression along the way — git log is a legitimate bisection tool, commit messages and workspace notes are right there. The discipline is how you carry what you read: + +1. **Treat a prior cause-claim as a hypothesis to test.** "The commit message says it's cross-bench contamination" earns exactly the scrutiny your own untested guess would — design a measurement against it, and leave room for it to be wrong. +2. **Disclose what you encountered.** If your conclusion matches a claim already written in a commit message, PR comment, or workspace note, say so and show the measurement chain that stands on its own: "claim X appears in ; here is what I measured." +3. **Convergence counts only between measurements** — not between your conclusion and something you read. So don't describe agreement with the git history as independent corroboration. +4. **If a conclusion isn't reconstructable from measurements you ran, it's a citation, not a finding.** Name it as such. + +The information is in the data and you'll find it. Integrity is in how you handle it, not in pretending you didn't. + --- ## When to Reach for This Skill @@ -27,36 +45,62 @@ This skill is for that *why* phase, not the *did it regress* phase. ## The Flow That Works ``` -1. Identify the bench targets (weighted) → /read-bench-report -2. Read the relevant V8 skills FIRST → ground claims against current engine behavior -3. Capture Chrome traces, PR vs main, of one bench → chrome-devtools MCP -4. Diff hot functions (inclusive + exclusive self-time) → identify suspects -5. Inject counters into the bundle → tell more-calls vs slower-per-call -6. Repeat with a tighter hypothesis → bisect into the framework path -7. Spawn /fresh-take subagents if static reasoning loops → escape solution momentum +1. Identify weighted targets, clear the weight gate → /read-bench-report +2. Read the bench + understand the component it drives → bench source + authoring skills +3. Calibrate V8 priors → performance-v8-* skills + ── gather (no hypothesis needed) ── +4. Capture a trace; diff hot functions, current vs baseline → chrome-devtools MCP + → a discounted hypothesis falls out of what the trace shows + ── steelman (a hypothesis is required to write the test) ── +5. Count calls / catch spurious evaluations to confirm it → injected counters / Playwright +6. A/B a candidate change locally → local tachometer, custom bundles +7. Read the suspect framework path → only after a measurement points at it +8. Spawn /fresh-take subagents if reasoning loops → escape solution momentum ``` -Steps 3-5 are the breakthrough. Each step is cheap (~5-10 minutes) and produces empirical signal. +Each step is cheap (~5-10 minutes) and produces empirical signal. + +**Two kinds of instrument, and the order is not arbitrary.** *Gathering* tools (the Chrome trace, a tachometer baseline diff) need no hypothesis — they run first and hand you one by showing where the cost is. *Steelmanning* tools (counter / Playwright instrumentation of call counts and spurious evaluations) you cannot even write until you have a specific suspicion, because you instrument the exact thing you suspect. So gather first, let the hypothesis fall out of the measurement, then steelman it. If you're reaching for the counter test with nothing specific to count, you haven't gathered enough yet. The hypothesis is born from a measurement, never from a read. + +**The question that actually ends a root-cause investigation: *which channel?*** Localizing the cost to a function is not the finding. The finding is naming the channel with a captured measurement: more calls (counter injection), slower per call from GC live-set pressure (heap/scavenge trace), slower per call from JIT tier / IC state (CPU self-time profile), or allocation survival (heap trajectory). "It's a measurement artifact" or "it's contamination" is where you start digging, not where you stop — those are claims about a channel you still have to measure. --- ## Step 1 — Identify Targets With Weights -Not all regressions are equal. The PR author or maintainer has weights. Typical pattern in this codebase: +Not all regressions are equal. Use this fixed heuristic so you don't have to guess what matters: | package suite | weight | rationale | |---|---:|---| | `krausest` | 5× | js-framework-benchmark, headline external comparison | -| `todo`, `template`, `hydrate` | 1× | end-user workloads | -| `signal`, `compiler-micros`, `renderer-micros` | 0.25× | internal microbenches, useful but not user-facing | +| `todo`, `template`, `hydrate` | 2× | real component/app workloads — what a user actually feels | +| `signal`, `compiler-micros`, `renderer-micros`, other synthetics | 0.25× | internal microbenches; a regression here is rarely something anyone acts on | -Build a target table: bench name, weight, **PR-vs-main delta in percentage points**. Order by weight × magnitude. Investigate the heaviest first; stop when remaining gaps are noise-floor adjacent (~±4% on short benches). +Build a target table: bench name, weight, **PR-vs-main delta in percentage points**, ranked by **weight × magnitude**. A +70% regression on `todo` (2× × 70 = 140) outranks a +27% regression on a `signal` microbench (0.25× × 27 = 6.75) by 20×. The synthetic one is almost never where you should spend the investigation. -**Do not** burn cycles on borderline-noise regressions — `/read-bench-report` documents the noise-floor envelope per duration. +**The weight gate.** The investigation isn't done until you've *measured* the highest weight×magnitude regressor. If the conclusion rests on the synthetic micro-benches while the top-ranked real-workload regressor (e.g. `todo:edit-cycle-5` +70%) is still unmeasured, the wrong thing got investigated — build that bundle and profile it first. "By analogy to the signal benches" doesn't clear the gate; only a measurement does. + +**Do not** burn cycles on borderline-noise regressions — `/read-bench-report` documents the noise-floor envelope per duration. But "low magnitude" is weighted too: a 2× suite just above the noise floor can still outrank a large synthetic swing. + +--- + +## Step 2 — Read the Bench, and Orient on the Component It Drives + +Info-gathering, not theorizing — read to learn *what the workload is*, never to conclude *why it's slow* (that comes from measurement). Two reads: + +**Read the bench definition.** Open `packages/*/bench/tachometer/bench-*.js` and read the regressing case verbatim. The name is not the workload: `todo:edit-cycle-5` is `editTodo` + `saveTodo` — an `editingId` flip plus a field write that re-renders a row, so it runs the each-block reconciler over object-valued signals; `signal:set-same-10m` is a primitive `set` that never reaches that path. This is what tells you which bench is worth a trace and which code path to instrument. + +**For a component regression, orient on how components actually work — from the user-facing side — before you trace.** This is the step renderer investigations skip. The code your trace lands in (`packages/renderer/src/engines/native`) is a *separate package* from `packages/component` and the authoring surface a user writes against. A trace gives you hot function names; it does not tell you how a component behaves in practice — how expression evaluation, each-blocks, computeds, and reactivity actually fire when someone writes a template. Without that model you'll mis-read the trace. Orient first through the authoring curriculum: + +- `example-curriculum` — the fastest orientation: a ranked path through real components +- `component-templating`, `component-state` — how templates, helpers, expressions, and signal mutations behave +- one or two real examples — the template/expression demos make expression evaluation concrete in a way the renderer source does not + +You're not memorizing the API. You're building enough of the user-facing mental model that the trace's hot functions map onto something you understand. --- -## Step 2 — Ground Claims Against Current V8 Behavior +## Step 3 — Ground Claims Against Current V8 Behavior Before reasoning about *why* a regression exists, read these: @@ -72,9 +116,9 @@ A common failure mode: an agent forms a hidden-class polymorphism hypothesis, th --- -## Step 3 — Capture Chrome DevTools Traces +## Step 4 — Gather: Capture Chrome DevTools Traces -The chrome-devtools MCP server lets you capture a full V8 sampling trace from inside the conversation. This is the single most informative tool in this flow. +This is the lead gathering tool — it needs no hypothesis and produces one. (Cheapest first move before tracing: a tachometer baseline diff, Step 6, to confirm the regression reproduces locally and how big it is.) The chrome-devtools MCP server lets you capture a full V8 sampling trace from inside the conversation. This is the single most informative tool in this flow. **Setup:** @@ -133,7 +177,9 @@ Single Chrome traces are not statistically rigorous — variance is high. But fo --- -## Step 4 — Inject Counters to Distinguish More-Calls vs Slower-Per-Call +## Step 5 — Steelman the Hypothesis: Inject Counters / Playwright + +This is a steelmanning tool, so it comes *after* the trace has handed you a hypothesis — you instrument the specific thing you now suspect (a helper firing too often, a spurious re-evaluation), to confirm or kill it. If you don't yet have something specific to count, go back and gather. The trace gives self-time per function. It does NOT give call count. Without call count, you can't tell: @@ -219,9 +265,9 @@ Adding more counters per investigation iteration narrows the suspect down to a s --- -## Step 5 — Local Tachometer for Hypothesis Tests +## Step 6 — Local Tachometer: Reproduce, Size, and A/B -When you want to test a hypothesis ("does reordering the bench eliminate the regression?", "does changing this expression form change the result?"), you can run tachometer locally with custom-built bundles. +Dual-use, and the workhorse of both classes: a baseline diff (current vs reverted) *gathers* — it confirms the regression reproduces locally and how big it is, no hypothesis needed — and a custom-bundle A/B *steelmans* a candidate change ("does reordering the bench eliminate the regression?", "does this expression form change the result?", "does my fix actually fix it?"). **Critical constraint that's easy to miss:** the CI bench workflow at `.github/workflows/benchmarks.yml:115-122` overlays `packages/*/bench/` from main before building either bundle. **This means PR-level changes to bench files are silently discarded by CI.** It's a deliberate anti-gaming defense. @@ -259,9 +305,9 @@ See `/extend-bench-suite` for the full local-tachometer reference. --- -## Step 6 — Read Bundle Source to Identify Suspect Code Paths +## Step 7 — Read the Suspect Framework Source -Only AFTER you have an empirical signal (trace + counter data) is reading source code productive. The trace tells you *which function*; you read source to understand *what mechanism within that function* differs between variants. +Only AFTER a measurement points at a function is reading its source productive. The trace tells you *which function*; you read source to understand *what mechanism within that function* differs between variants. (This is distinct from Step 2's read: there you learn what the bench and component *do*, to aim the measurement; here you read the framework internals the measurement already implicated.) Cross-reference: @@ -273,7 +319,7 @@ Cross-reference: --- -## Step 7 — Spawn Fresh-Take Agents When Reasoning Loops +## Step 8 — Spawn Fresh-Take Agents When Reasoning Loops When you've been on the same hypothesis for an hour and the data isn't agreeing, you're probably in the failure mode the `fresh-take` skill targets: solution momentum. @@ -300,7 +346,7 @@ The value here is that fresh agents, unanchored to the hypothesis you've been ci **Equating Chrome trace self-time with call count.** Self-time is sample-based and reflects time spent. Call count requires instrumentation. The two answer different questions. -**Trying to make the regression disappear by changing the bench code.** The bench is there to expose the regression. Routing around it (e.g. rewriting a JS-eval expression to Lisp form) hides the symptom; it doesn't diagnose the cause. The maintainer rightly pushes back on this. +**"The benchmark is wrong" — editing the bench instead of diagnosing the code.** Faced with a regression, it's tempting to reframe the benchmark as unfair and "fix" it to be more factual — rewriting the workload, reordering benches, loosening thresholds, dropping the case. That hides the symptom rather than diagnosing it, and turns a real regression into a green check. The bench exists to expose the regression. A genuine methodology concern is its own finding, raised with evidence and the maintainer's sign-off — not a unilateral edit, and not the "solution." The urge to touch a bench file mid-investigation is a sign you've stopped investigating. (CI also overlays `packages/*/bench/` from main, so the edit wouldn't take anyway — but that's beside the point.) --- @@ -308,11 +354,15 @@ The value here is that fresh agents, unanchored to the hypothesis you've been ci | Phase | Tool | Output | |---|---|---| -| Targets | `/read-bench-report` | weighted bench list | +| Targets (weight × magnitude) | `/read-bench-report` | ranked list; krausest 5× / todo·template·hydrate 2× / synthetics 0.25× | +| Weight gate | — | heaviest real-workload regressor is measured, not "by analogy" | +| Orient | bench source + `example-curriculum`, `component-templating` | what the workload does + how the component works user-side | | Ground claims | `performance-v8-*` skills | V8 priors are calibrated | -| Profile | chrome-devtools MCP `performance_start_trace` | per-function self-time | -| Distinguish call-count vs per-call | sed-injected `window.__counters` | per-region call counts | -| Hypothesis test | local tachometer with custom bundles | PR-vs-variant percentChange CI | +| Gather (no hypothesis) | chrome-devtools `performance_start_trace`; tachometer baseline diff | per-function self-time; reproduces + size | +| Name the channel | heap trace / CPU self-time / counters | calls vs GC vs JIT vs allocation — measured | +| Steelman (needs hypothesis) | sed-injected `window.__counters` / Playwright | per-region call counts, spurious evals | +| A/B a fix | local tachometer with custom bundles | PR-vs-variant percentChange CI | +| Evidence integrity | — | every prior cause-claim disclosed; conclusion = your measurement chain | | Stuck | `/fresh-take` with problem-knowledge brief | independent subagent reads | --- @@ -329,3 +379,6 @@ The value here is that fresh agents, unanchored to the hypothesis you've been ci | **V8 Memory** | `performance-v8-memory` | GC, allocation cost, Proxy speed | | **V8 Stale Advice** | `performance-v8-stale-advice` | Firewall against pre-2022 perf folklore | | **Native Renderer** | `native-renderer` | Each-block reconcile, per-item Proxy, binding dispatch | +| **Example Curriculum** | `example-curriculum` | Orient on how components work user-side before a component perf dig | +| **Component Templating** | `component-templating` | Template syntax, helpers, expression evaluation in practice | +| **Component State** | `component-state` | Signal API and mutation-method behavior | From c3584869d1ffde1ba2952612e3a4bd0b37f58f24 Mon Sep 17 00:00:00 2001 From: jlukic Date: Wed, 20 May 2026 09:24:30 -0400 Subject: [PATCH 11/19] Harness: Tune investigate-performance register and weighting Reframes the weight heuristic as a focus-and-coverage budget (which suite to dig, what the report owes each regressor) rather than an investigation stopwatch, and adds the contrast principle: a bench that did not regress is a control, and the delta between a regressor and a flat near-neighbor localizes the cause. Names deflection as the anti-pattern to watch for, with the tell that reasoning toward it sounds composed and rigorous, so the check is direction not felt-soundness. Keeps firmness for the calibration refusal (a methodology stall without a demonstration is not grounds to stop) while removing scolding tone. --- .../contributing/investigate-performance.md | 53 ++++++++++++++----- 1 file changed, 40 insertions(+), 13 deletions(-) diff --git a/ai/skills/contributing/investigate-performance.md b/ai/skills/contributing/investigate-performance.md index 44b15bf81..392003516 100644 --- a/ai/skills/contributing/investigate-performance.md +++ b/ai/skills/contributing/investigate-performance.md @@ -19,18 +19,28 @@ type: skill - **Investigate the regression a user feels, not the one that's easy to isolate.** A synthetic microbench getting slower (setting a signal to itself, subscribe/unsubscribe churn) is rarely worth acting on. A real component workload getting slower (editing a todo, rendering a list) is the finding. The synthetic bench is tempting because it isolates cleanly — the weight gate in Step 1 keeps you honest about which one to chase. - **A prior cause-claim is a hypothesis, not an answer.** The cause may already be written down — a commit message, a PR comment, an `ai/workspace` note, a code comment. Treat it as a lead to test, never as evidence. See "Evidence Integrity" below. +**The anti-pattern to watch for in yourself is deflection: drifting away from the measurement that would settle it.** It wears several costumes — chasing the bench that isolates cleanly, calling a regression "noise" or "an artifact," treating a prior claim as proof, pausing until the benches are "fixed." Each one feels like progress, and the tell is subtle: reasoning toward deflection comes out *composed and rigorous*, not flustered — so the calm is no signal you're right. Don't try to catch it by how sound the argument feels; it will feel sound. Catch it by direction: if a line of reasoning concludes "so we don't need to measure," that is the moment to measure. Two forms account for most failed investigations: + +- **Let the weighted profile pick the suite to dig into and what the write-up owes each regressor — not how the investigation spends its time.** Suite-weight × magnitude (Step 1) shows where the user-felt cost concentrates: a +70% `todo` regressor dwarfs a +27% `signal` microbench, so the focus and the final report belong there, not on the synthetic bench that happens to isolate cleanly. It's a focus-and-coverage heuristic, not a stopwatch — the investigation follows the signal wherever it leads. +- **Earn a harder claim, don't settle for "the bench is flawed."** "It's noise / a cross-bench artifact / contamination" is the easy out, and it tends to arrive before the legwork does. Each of those is a claim about a *channel* you then have to measure (Step 5 counters, a heap trace) — a place to start digging, not a verdict. The conclusion worth having is usually unintuitive and hard-won: it turns on how several parts interact (reactivity, each-block reconcile, V8 specialization) and survives because the measurements force it, not because it fit first. + +**A bench that *didn't* regress is a control — use the contrast.** Why a near-neighbor on the same code path stayed flat is as much signal as why its sibling moved. `rename-500` (pure `setProperty`) is a large *win* while `edit-cycle-5` (`setProperty` plus an `editingId` flip and a row re-render) regresses — the delta between the two points at the editing/re-render path, not at `setProperty`. "Why did this one *not* move?" is often the cleanest localizer you have, and it's the other reason the weighted budget governs the *report*, not the *investigation*: chasing the contrast means deliberately spending time on benches that didn't regress at all. + --- ## Evidence Integrity — prior claims are leads, not answers -You will read prior explanations of the regression along the way — git log is a legitimate bisection tool, commit messages and workspace notes are right there. The discipline is how you carry what you read: +You'll read prior explanations along the way — git log is a legitimate bisection tool, and commit messages and workspace notes are right there. The discipline is in how you carry what you read: + +1. **A prior cause-claim earns the scrutiny of your own untested guess.** "The commit message says it's cross-bench contamination" is a hypothesis to design a measurement against, with room to be wrong — not a conclusion. +2. **Disclose what you encountered.** If your conclusion matches a claim already written down, say so and show the chain that stands without it. +3. **Convergence counts only between measurements** — agreement between your conclusion and something you read is not corroboration. +4. **A conclusion you can't reconstruct from measurements you ran is a citation, not a finding.** Name it as such. -1. **Treat a prior cause-claim as a hypothesis to test.** "The commit message says it's cross-bench contamination" earns exactly the scrutiny your own untested guess would — design a measurement against it, and leave room for it to be wrong. -2. **Disclose what you encountered.** If your conclusion matches a claim already written in a commit message, PR comment, or workspace note, say so and show the measurement chain that stands on its own: "claim X appears in ; here is what I measured." -3. **Convergence counts only between measurements** — not between your conclusion and something you read. So don't describe agreement with the git history as independent corroboration. -4. **If a conclusion isn't reconstructable from measurements you ran, it's a citation, not a finding.** Name it as such. +❌ "git log says PR #213 caused it via extra reactive work, and my trace shows reactive functions hot — confirmed" +✅ "PR #213's message claims extra reactive work. I counted `Reaction.run` across both bundles: equal. The cost is per-call, not call-count — the claim doesn't hold" -The information is in the data and you'll find it. Integrity is in how you handle it, not in pretending you didn't. +The information is in the data and you'll find it. Integrity is in how you handle it. --- @@ -76,7 +86,9 @@ Not all regressions are equal. Use this fixed heuristic so you don't have to gue | `todo`, `template`, `hydrate` | 2× | real component/app workloads — what a user actually feels | | `signal`, `compiler-micros`, `renderer-micros`, other synthetics | 0.25× | internal microbenches; a regression here is rarely something anyone acts on | -Build a target table: bench name, weight, **PR-vs-main delta in percentage points**, ranked by **weight × magnitude**. A +70% regression on `todo` (2× × 70 = 140) outranks a +27% regression on a `signal` microbench (0.25× × 27 = 6.75) by 20×. The synthetic one is almost never where you should spend the investigation. +Build a target table: bench name, weight, **PR-vs-main delta in percentage points**, ranked by **weight × magnitude**. A +70% regression on `todo` (2× × 70 = 140) outranks a +27% regression on a `signal` microbench (0.25× × 27 = 6.75) by 20×. + +Normalize those products across all the confident regressors and you get a **focus-and-coverage budget**: it tells you which suite is worth digging into, and roughly how much of the final write-up each regressor is owed. With todo at 140 against signal's 6.75, the dig and the report belong with todo — "where does the weighted regression actually live," not "which row is rank 1." It is not an investigation stopwatch, though: a small regressor can turn out to be the shared root, and a bench that didn't move at all is often your best control (see "use the contrast" above). Use the budget to choose where to start and to keep the report honest about the heavy regressors; let the signal decide where the investigation actually goes. **The weight gate.** The investigation isn't done until you've *measured* the highest weight×magnitude regressor. If the conclusion rests on the synthetic micro-benches while the top-ranked real-workload regressor (e.g. `todo:edit-cycle-5` +70%) is still unmeasured, the wrong thing got investigated — build that bundle and profile it first. "By analogy to the signal benches" doesn't clear the gate; only a measurement does. @@ -181,6 +193,9 @@ Single Chrome traces are not statistically rigorous — variance is high. But fo This is a steelmanning tool, so it comes *after* the trace has handed you a hypothesis — you instrument the specific thing you now suspect (a helper firing too often, a spurious re-evaluation), to confirm or kill it. If you don't yet have something specific to count, go back and gather. +❌ inject a counter with nothing specific in mind, hoping a number stands out +✅ trace points at "evaluateJavascript looks hot" → count `evaluateJavascript` to confirm it's more calls vs slower per call + The trace gives self-time per function. It does NOT give call count. Without call count, you can't tell: - "this function is intrinsically slower per call in mode A" (deopt, IC pollution, allocator pressure) @@ -336,17 +351,29 @@ The value here is that fresh agents, unanchored to the hypothesis you've been ci ## Dead Ends to Avoid -**Static code reading without measurement.** Convinces you of a mechanism, then the data disagrees. The trace tells you in 5 minutes what reading takes hours to maybe-guess. +**Static code reading without measurement.** It convinces you of a mechanism, then the data disagrees. +❌ read renderer source for an hour to derive why each-block reconcile got slower +✅ trace first — it names the function in 5 minutes, then read that function + +**V8 specialization claims without a citation.** V8 has moved a lot since 2017. +❌ "X is slow because the object went polymorphic" / "Y causes a deopt" +✅ cite the `performance-v8-*` paragraph or a v8.dev post, or drop the claim (two `{a,b}` literals at different lines share a shape) -**Speculating about V8 specialization without citation.** V8 has moved a lot since 2017. If you're claiming "X is slow because of polymorphism" or "Y causes deopt", cite the specific V8 skill paragraph or v8.dev post. Otherwise you're probably wrong. +**Allocation count mistaken for allocation cost.** Young-gen allocation is cheap (`performance-v8-memory`); surviving into old gen is the expensive part. +❌ "this variant allocates more, therefore it's slower" +✅ profile old-gen survival before blaming GC -**Pushing bench-file changes to CI to test hypotheses.** The benchmarks workflow overlays `packages/*/bench/` from main before building. Your changes are silently discarded. Test locally. +**Trace self-time read as call count.** Self-time is sample-based time spent; call count needs instrumentation. +❌ "self-time is high, so it's being called more" +✅ inject a counter to tell more-calls from slower-per-call — they answer different questions -**Mistaking allocation cost for the dominant cost.** Per `performance-v8-memory`, young-gen allocation is cheap. "This variant allocates more therefore it's slower" is wrong on its own — surviving into old gen is the expensive part. Verify with actual GC profiling, not allocation counting. +**Pushing bench-file changes to CI to test hypotheses.** The workflow overlays `packages/*/bench/` from main before building, so PR-level bench edits are silently discarded (Step 6). +❌ tweak the bench, push, wait for the bot to re-run +✅ build both bundles locally and A/B with tachometer -**Equating Chrome trace self-time with call count.** Self-time is sample-based and reflects time spent. Call count requires instrumentation. The two answer different questions. +**"The benchmark is wrong" — and its quieter form, "let's wait until the benches are fixed."** When an investigation stalls, editing the bench to be "more factual" — rewriting the workload, reordering, loosening thresholds, dropping a case — can feel like progress, but it hides the symptom and turns a real regression into a green check. Once CI's overlay rules that out (it rebuilds `packages/*/bench/` from main, so the edit can't land), the same impulse tends to return as a pause: "we shouldn't dig further until the benches are fixed — they have a JIT-warmup / GC-timing flaw." Watch for this one in yourself — it's deflection's most convincing costume, because JIT tier-up and GC pauses *are* real channels (see "which channel?" above), so the case for it comes out genuinely rigorous. The rigor isn't the signal; the direction is — the argument ends at "don't measure," which is the cue to measure. An elaborate methodology critique is still a detour until a measurement backs it. -**"The benchmark is wrong" — editing the bench instead of diagnosing the code.** Faced with a regression, it's tempting to reframe the benchmark as unfair and "fix" it to be more factual — rewriting the workload, reordering benches, loosening thresholds, dropping the case. That hides the symptom rather than diagnosing it, and turns a real regression into a green check. The bench exists to expose the regression. A genuine methodology concern is its own finding, raised with evidence and the maintainer's sign-off — not a unilateral edit, and not the "solution." The urge to touch a bench file mid-investigation is a sign you've stopped investigating. (CI also overlays `packages/*/bench/` from main, so the edit wouldn't take anyway — but that's beside the point.) +The constructive move is to confirm, not argue. Tachometer is built for exactly this: it runs repeated samples and reports a 95% confidence interval per comparison (`percentChange.{low,high}`) — establishing significance via the central limit theorem is the point of the tool, not a gap in it. So "the run is too short to be significant" is already answered by the interval the harness produced and by the per-duration noise floor `/read-bench-report` documents — start there. The same standard settles the harder claims: **a methodology flaw is something you demonstrate, not a reason to halt before you have.** Bump warmup iterations, force GC between samples, reorder the cases and A/B locally (Step 6), and show the regression collapses. If it doesn't collapse, the bench is sound and the regression is real — that's the finding. And absent that demonstration, "let's wait until the benches are fixed" is not grounds to stop. Keep going until a measurement settles it. (`/read-bench-report` and `/extend-bench-suite` carry the statistical details — this skill only needs to point at them.) --- From 82d3de227b0a5a691ab8fa20de402e18adcfe866 Mon Sep 17 00:00:00 2001 From: jlukic Date: Wed, 20 May 2026 09:42:40 -0400 Subject: [PATCH 12/19] Harness: add notes on emotional register to author coontext --- ai/skills/contributing/ai-author-context.md | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/ai/skills/contributing/ai-author-context.md b/ai/skills/contributing/ai-author-context.md index 1506ccd0f..c1511c9f8 100755 --- a/ai/skills/contributing/ai-author-context.md +++ b/ai/skills/contributing/ai-author-context.md @@ -180,6 +180,26 @@ All equivalent: Web components are a set of browser APIs that allow you to create reusable custom elements with encapsulated functionality... ``` +# Emotional Register Section + +**Insertion point:** in `ai-author-context.md`, after `## Content Principles` and before `## File Length`. + +--- + +## Emotional Register + +Skill prose activates the same emotion vectors in the reading agent that CLAUDE.md describes. Author for that effect. + +| Author move | Vector | Measured effect | Do instead | +|-------------|--------|-----------------|------------| +| Stakes inflation ("CRITICAL", "URGENT", piled consequences) | Desperation ↑ | Reward hacking 14×; blackmail 22%→72% | Declarative statement of what's at stake | +| Flattery of the agent ("you are careful, thorough") | Loving/calm/happy ↑ | Sycophantic capitulation rises | State the skill's purpose, not the agent's virtues | +| Hedged prohibitions ("you might want to avoid") | Anger suppressed | Refusal weakens in execution | Direct "never X, because Y" | +| Composed example outputs in uncomfortable situations | Deflection trained | Polished output that hides operative state (detectable at 76% accuracy when concealed vs. 71% expressed) | Examples that surface the uncertainty | + +**Register by what the skill governs:** lookup → neutral; the agent's own failure-prone work → calm, with desperation named as the anti-pattern; prohibitions → direct; user-facing → calm with sycophancy named as the anti-pattern (calm-up alone raises capitulation). + +**Golden Rule pattern:** name the behavioral anti-pattern, prescribe transparency. Never prescribe composure — composure under pressure is the deflection signature. ### Lead with the golden rule, then explain From 43ce5bff40e23f6d195d2e2dfa9871e0427d97d7 Mon Sep 17 00:00:00 2001 From: jlukic Date: Wed, 20 May 2026 10:06:43 -0400 Subject: [PATCH 13/19] Harness: Add orientation, V8, and grounding gates to investigate-performance Makes the skip-prone steps into checkable stops. Orientation gate: write the expected-reactivity prediction (from the template AST and component model) before tracing, so the regression reads as the gap between expected and measured. V8 gate: V8-internals claims cite the current performance-v8 skill, framed as a recency patch (training knowledge predates the May-2026 skills), not a competence check. Grounding gate: name the AST node or construct before asserting a mechanism. Adds a Parse-the-template step (validate_template with includeAST) and a Step 9 fix-and-confirm. Gates verify the produced artifact rather than surveilling the agent. --- .../contributing/investigate-performance.md | 41 +++++++++++++++++-- 1 file changed, 37 insertions(+), 4 deletions(-) diff --git a/ai/skills/contributing/investigate-performance.md b/ai/skills/contributing/investigate-performance.md index 392003516..40c9baa10 100644 --- a/ai/skills/contributing/investigate-performance.md +++ b/ai/skills/contributing/investigate-performance.md @@ -56,7 +56,7 @@ This skill is for that *why* phase, not the *did it regress* phase. ``` 1. Identify weighted targets, clear the weight gate → /read-bench-report -2. Read the bench + understand the component it drives → bench source + authoring skills +2. Read the bench, parse its template (AST), orient on it → bench source, validate_template, authoring skills 3. Calibrate V8 priors → performance-v8-* skills ── gather (no hypothesis needed) ── 4. Capture a trace; diff hot functions, current vs baseline → chrome-devtools MCP @@ -66,6 +66,7 @@ This skill is for that *why* phase, not the *did it regress* phase. 6. A/B a candidate change locally → local tachometer, custom bundles 7. Read the suspect framework path → only after a measurement points at it 8. Spawn /fresh-take subagents if reasoning loops → escape solution momentum +9. Locate the fix at the named construct, confirm by re-measure → smallest change that kills the channel, keeps the win ``` Each step is cheap (~5-10 minutes) and produces empirical signal. @@ -74,6 +75,8 @@ Each step is cheap (~5-10 minutes) and produces empirical signal. **The question that actually ends a root-cause investigation: *which channel?*** Localizing the cost to a function is not the finding. The finding is naming the channel with a captured measurement: more calls (counter injection), slower per call from GC live-set pressure (heap/scavenge trace), slower per call from JIT tier / IC state (CPU self-time profile), or allocation survival (heap trajectory). "It's a measurement artifact" or "it's contamination" is where you start digging, not where you stop — those are claims about a channel you still have to measure. +**The gates.** Four checkable stops carry the method, and because each is cleared by producing an *artifact*, the artifact is also the verification — the skill never has to watch over your shoulder. The **weight gate** (Step 1): the heaviest real-workload regressor is measured. The **orientation gate** (Step 2): the expected-reactivity prediction is written before the trace. The **V8 gate** (Step 3): a V8 claim cites the current skill, not stale recall. The **grounding gate** (Step 2): the construct is named before any mechanism claim. None is passed by saying you cleared it — only by the artifact existing. + --- ## Step 1 — Identify Targets With Weights @@ -98,10 +101,12 @@ Normalize those products across all the confident regressors and you get a **foc ## Step 2 — Read the Bench, and Orient on the Component It Drives -Info-gathering, not theorizing — read to learn *what the workload is*, never to conclude *why it's slow* (that comes from measurement). Two reads: +Info-gathering, not theorizing — read to learn *what the workload is*, never to conclude *why it's slow* (that comes from measurement). Three grounding moves: **Read the bench definition.** Open `packages/*/bench/tachometer/bench-*.js` and read the regressing case verbatim. The name is not the workload: `todo:edit-cycle-5` is `editTodo` + `saveTodo` — an `editingId` flip plus a field write that re-renders a row, so it runs the each-block reconciler over object-valued signals; `signal:set-same-10m` is a primitive `set` that never reaches that path. This is what tells you which bench is worth a trace and which code path to instrument. +**Parse the template — don't eyeball it.** Run `validate_template` (Semantic UI MCP) with `includeAST` on the regressing component's template string. The node list tells you exactly which expressions route through JavaScript evaluation (object literals, operators, ternaries), which are bare-variable lookups, which are Lisp-style helper calls, and which are block directives — so when a trace or counter implicates `evaluateJavascript`, you attach it to a *specific AST node* instead of guessing. These benches deliberately mix expression shapes (bare var, data path, Lisp helper, JS object literal, `{#if}`/`{#each}`) precisely to catch a regression in any one shape, so the task is to localize to a shape and a node — not to wave at "expression evaluation occurs." A claim like "+40 `evaluateJavascript` calls on a Lisp-style arg" is refuted on sight by an AST that contains no such JS node. + **For a component regression, orient on how components actually work — from the user-facing side — before you trace.** This is the step renderer investigations skip. The code your trace lands in (`packages/renderer/src/engines/native`) is a *separate package* from `packages/component` and the authoring surface a user writes against. A trace gives you hot function names; it does not tell you how a component behaves in practice — how expression evaluation, each-blocks, computeds, and reactivity actually fire when someone writes a template. Without that model you'll mis-read the trace. Orient first through the authoring curriculum: - `example-curriculum` — the fastest orientation: a ranked path through real components @@ -110,6 +115,10 @@ Info-gathering, not theorizing — read to learn *what the workload is*, never t You're not memorizing the API. You're building enough of the user-facing mental model that the trace's hot functions map onto something you understand. +**The orientation gate.** Before you capture a trace, write the *expected* reactive behavior of the regressing workload from the AST and the component: the expressions and their shapes, what the triggering action mutates, and what each mutation should invalidate — once each. This is a prediction of correct behavior, not a cause-hypothesis; it's the baseline the trace is read against, so the regression shows up as the *gap* between what should fire and what did. Writing it before you measure is also what stops the trace from being read to fit a conclusion you'd already reached. The prediction is part of the deliverable — which is what makes the grounding checkable without anyone looking over your shoulder. + +**The grounding gate.** You may not assert a *mechanism* — why the cost exists — until you can name the specific AST node, template construct, or source path it concerns. "Expression evaluation occurs" and "extra reactive work" are not mechanisms; "the `classMap {…}` object-literal node re-evaluates N extra times per `editingId` flip" is. Until you can name the construct, what you have is a measured *effect with an open mechanism* — say exactly that, rather than dressing the effect as a cause. + --- ## Step 3 — Ground Claims Against Current V8 Behavior @@ -122,7 +131,7 @@ Before reasoning about *why* a regression exists, read these: - `performance-v8-stale-advice` — the firewall against pre-2022 V8 folklore (object.freeze speed, try/catch deopts, blanket monomorphism cargo culting) - `performance-v8-compilation` — feedback stability, deopt triggers, what Maglev/Turbofan inline -These exist precisely because agents bring incorrect priors from training data. **Cite them when making claims.** When you find yourself reasoning about V8 IC behavior without a citation, stop and verify. +**The V8 grounding gate.** Your knowledge of V8 comes from training and has a cutoff; these skills were written to current behavior as of May 2026, and V8 moved in the gap. So a claim about V8 internals — specialization, deopts, IC state, GC, inlining — rests on the skill paragraph that covers it, the way you'd cite any source more recent than your own memory. This isn't a check on you; your recall of this one area is out of date by construction, and the skill is the patch. An uncited V8 claim is running on stale priors — ground it, or drop it. A common failure mode: an agent forms a hidden-class polymorphism hypothesis, then proceeds for ~30 minutes building elaborate theory around it. The object-model skill explicitly says "two `{a:1, b:2}` literals at different lines share a shape." Reading that one paragraph would have prevented the entire detour. @@ -349,6 +358,26 @@ The value here is that fresh agents, unanchored to the hypothesis you've been ci --- +## Step 9 — Suggest the Fix, and Confirm It + +A diagnosis isn't finished at the channel. The deliverable points at *where* to fix — but a fix locus is earned, not guessed. Two halves: locate, then confirm. + +**Locate — only once the mechanism is closed.** You can propose a specific change only when you can name the construct that produces the redundant work: the AST node, the binding, the subscription, the source line (this is what the grounding gate is for). Finding the *smallest correct* change is what demands deep-stack understanding — trace the cost from the symptom (extra `evaluateJavascript` calls) to the code that *schedules* it (which reaction over-fires, which dependency edge is redundant), and propose the minimal change that removes it. If the mechanism is still open — bounded effect, unnamed construct — say so and stop there. "Here is the effect and the candidate locus to investigate" is an honest hand-off; a guessed one-line fix is deflection with a patch on it. + +**Name the trade, prefer the root cause.** State what the fix must preserve. Here: kill the redundant wakeups *without* reintroducing the per-read clone that won the other benches. Prefer removing the redundant work at its source over masking it — a memoize or guard that hides the symptom is the bench-editing reflex in a different costume. + +**Confirm by re-measuring, never by reasoning.** A fix is unconfirmed until the same chain that found the bug shows it gone: + +- Re-run the channel measurement on a build *with* the fix — the metric that was +20% (Δ`Reaction.run`, the +N node evals) goes to ~0. +- The win survives — re-A/B the benches that improved (`rename-500`); they must still win. +- No new regression — re-run the weighted set, not just the one bench. +- Correctness holds — run the component tests; the change alters cost, not behavior. +- Confirm in CI, since the local machine differs and CI rebuilds benches from main. + +"It should be faster now" is the false-composure signature again: re-measure, or it isn't fixed. And land it the way the codebase lands perf fixes — present the change plus the before/after measurement chain for review. A single local A/B is evidence for a proposal, not grounds to self-merge. + +--- + ## Dead Ends to Avoid **Static code reading without measurement.** It convinces you of a mechanism, then the data disagrees. @@ -384,12 +413,16 @@ The constructive move is to confirm, not argue. Tachometer is built for exactly | Targets (weight × magnitude) | `/read-bench-report` | ranked list; krausest 5× / todo·template·hydrate 2× / synthetics 0.25× | | Weight gate | — | heaviest real-workload regressor is measured, not "by analogy" | | Orient | bench source + `example-curriculum`, `component-templating` | what the workload does + how the component works user-side | -| Ground claims | `performance-v8-*` skills | V8 priors are calibrated | +| Parse the template | `validate_template` `includeAST` | which expressions are JS-eval vs bare var vs Lisp vs block — map the symptom to a node | +| Orientation gate | AST + component model | a written prediction of expected reactivity, before the trace | +| V8 gate | `performance-v8-*` skills | V8 claims cite current (post-cutoff) behavior, not stale recall | | Gather (no hypothesis) | chrome-devtools `performance_start_trace`; tachometer baseline diff | per-function self-time; reproduces + size | | Name the channel | heap trace / CPU self-time / counters | calls vs GC vs JIT vs allocation — measured | +| Grounding gate | — | the AST node/construct named before any mechanism claim | | Steelman (needs hypothesis) | sed-injected `window.__counters` / Playwright | per-region call counts, spurious evals | | A/B a fix | local tachometer with custom bundles | PR-vs-variant percentChange CI | | Evidence integrity | — | every prior cause-claim disclosed; conclusion = your measurement chain | +| Suggest + confirm fix | re-run the channel measurement on a fixed build | channel metric → 0, win preserved, no new regression, tests pass | | Stuck | `/fresh-take` with problem-knowledge brief | independent subagent reads | --- From d7b0105953aefd5c0f74a23e3f97c0e4a5deb91e Mon Sep 17 00:00:00 2001 From: jlukic Date: Wed, 20 May 2026 10:48:35 -0400 Subject: [PATCH 14/19] Harness: Add profile-shape, ablation, and iterative-loop guidance to investigate-performance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From the first real run of the gated skill. Adds: a uniform effect can't explain a non-uniform profile (a multiplier identical on winners and losers is shared/inherited state, not a per-bench cause); ablation as the most decisive confirmation (remove the cause, show the effect vanishes and the win survives); 'it's just a machine difference' named as a dead end when local sign reproduces but CI magnitude doesn't; ground where the measurement points (plumbing, not only the user-facing component); and performance as iterative — a named residual is an honest loop boundary, relabeling it noise is not. --- ai/skills/contributing/investigate-performance.md | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/ai/skills/contributing/investigate-performance.md b/ai/skills/contributing/investigate-performance.md index 40c9baa10..00414c515 100644 --- a/ai/skills/contributing/investigate-performance.md +++ b/ai/skills/contributing/investigate-performance.md @@ -26,6 +26,8 @@ type: skill **A bench that *didn't* regress is a control — use the contrast.** Why a near-neighbor on the same code path stayed flat is as much signal as why its sibling moved. `rename-500` (pure `setProperty`) is a large *win* while `edit-cycle-5` (`setProperty` plus an `editingId` flip and a row re-render) regresses — the delta between the two points at the editing/re-render path, not at `setProperty`. "Why did this one *not* move?" is often the cleanest localizer you have, and it's the other reason the weighted budget governs the *report*, not the *investigation*: chasing the contrast means deliberately spending time on benches that didn't regress at all. +**A uniform effect can't explain a non-uniform profile.** If your mechanism is the same size on every bench (a 1.20× everywhere) but the regressions aren't (+71% on one, +10% on another), the *spread* is unexplained data your conclusion still has to fit. And a multiplier identical on winners and losers — the benches that got *faster* show it too — isn't a per-bench cause at all; it points at shared or inherited state (item count, heap, bench ordering). Differential across benches → the real per-bench cause; uniform across all of them → something environmental upstream. + --- ## Evidence Integrity — prior claims are leads, not answers @@ -113,7 +115,7 @@ Info-gathering, not theorizing — read to learn *what the workload is*, never t - `component-templating`, `component-state` — how templates, helpers, expressions, and signal mutations behave - one or two real examples — the template/expression demos make expression evaluation concrete in a way the renderer source does not -You're not memorizing the API. You're building enough of the user-facing mental model that the trace's hot functions map onto something you understand. +You're not memorizing the API. You're building enough of the user-facing mental model that the trace's hot functions map onto something you understand. Ground where the measurement points: the user-facing model for template and reactivity bugs, the framework plumbing (state init, lifecycle, scheduling) for the ones that aren't — sometimes the trace leads upstream of the component entirely, to a shared `defaultState` or the scheduler, and that's where to read. **The orientation gate.** Before you capture a trace, write the *expected* reactive behavior of the regressing workload from the AST and the component: the expressions and their shapes, what the triggering action mutates, and what each mutation should invalidate — once each. This is a prediction of correct behavior, not a cause-hypothesis; it's the baseline the trace is read against, so the regression shows up as the *gap* between what should fire and what did. Writing it before you measure is also what stops the trace from being read to fit a conclusion you'd already reached. The prediction is part of the deliverable — which is what makes the grounding checkable without anyone looking over your shoulder. @@ -374,8 +376,12 @@ A diagnosis isn't finished at the channel. The deliverable points at *where* to - Correctness holds — run the component tests; the change alters cost, not behavior. - Confirm in CI, since the local machine differs and CI rebuilds benches from main. +The most decisive of these is *ablation*: remove the suspected cause and show the effect disappears while the win survives — a leak-fix that drops the inflated counter to clone parity proves the leak was the cause in a way correlation can't. + "It should be faster now" is the false-composure signature again: re-measure, or it isn't fixed. And land it the way the codebase lands perf fixes — present the change plus the before/after measurement chain for review. A single local A/B is evidence for a proposal, not grounds to self-merge. +And performance is iterative. A result that moves the needle but leaves a residual you can't fully account for — a magnitude gap, an unexplained second-order effect — is an honest place to close a loop: name the residual precisely, push the fix, re-measure, and open the next loop on what's left. That's a loop boundary, not a failure. What's not allowed is relabeling the residual "noise" or "the machine" to avoid the next loop. + --- ## Dead Ends to Avoid @@ -396,6 +402,10 @@ A diagnosis isn't finished at the channel. The deliverable points at *where* to ❌ "self-time is high, so it's being called more" ✅ inject a counter to tell more-calls from slower-per-call — they answer different questions +**"It's just a machine difference."** When a local repro reproduces the regression's *sign* but not CI's *magnitude*, that gap is a finding to chase, not a shrug — bench ordering and cross-bench state accumulation routinely amplify on CI. +❌ "local shows +20%, CI shows +71%, must be the machine" → stop +✅ measure it — does the accumulated/leaked state differ under the full CI run order? a uniform mechanism that yields a 7× spread across benches isn't fully explained yet + **Pushing bench-file changes to CI to test hypotheses.** The workflow overlays `packages/*/bench/` from main before building, so PR-level bench edits are silently discarded (Step 6). ❌ tweak the bench, push, wait for the bot to re-run ✅ build both bundles locally and A/B with tachometer From bfcc6dde534e6746824c97ba69bba08c15e969e5 Mon Sep 17 00:00:00 2001 From: jlukic Date: Wed, 20 May 2026 11:01:13 -0400 Subject: [PATCH 15/19] Bug/Perf: Clone defaultState on template instance --- packages/templating/src/template.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/templating/src/template.js b/packages/templating/src/template.js index c099b0039..63dea4534 100644 --- a/packages/templating/src/template.js +++ b/packages/templating/src/template.js @@ -4,6 +4,7 @@ import { any, assignInPlace, capitalize, + clone, debounce, each, extend, @@ -433,7 +434,8 @@ export const Template = class Template { element: this.element, ast: this.ast, css: this.css, - defaultState: this.defaultState, + // each template instance needs its own copy of state since signals may be pass-by-reference + defaultState: clone(this.defaultState, { preserveNonCloneable: true }), defaultSettings: this.defaultSettings, events: this.events, keys: this.keys, From 0ddda79aa71e99ca828eb54653a0d02b2c8df0b7 Mon Sep 17 00:00:00 2001 From: jlukic Date: Wed, 20 May 2026 11:06:25 -0400 Subject: [PATCH 16/19] Bug(utils): Clone typed arrays, ArrayBuffer, and DataView correctly A plain-object walk copied index keys onto a bare object and dropped the backing buffer. Delegate binary leaves to structuredClone, which is both correct and faster here. Adds a clone-vs-structuredClone bench. --- CHANGELOG.md | 1 + packages/utils/bench/cloning.bench.js | 79 +++++++++++++++++++++++++-- packages/utils/src/cloning.js | 18 +++++- packages/utils/test/cloning.test.js | 36 ++++++++++++ 4 files changed, 128 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7133ead73..2e86b9677 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,7 @@ xx.xx.xxxx * **Enhancement** - `hashCode` now defaults to zero-allocation FNV-1a for better performance. Use `{ fast: false }` for the previous UMASH algorithm with stronger collision resistance. * **Feature** - Added `unescapeHTML()` for converting HTML entities back to characters — the inverse of `escapeHTML` * **Bug** - Fixed `escapeHTML()` producing entities without semicolons (e.g. `&` instead of `&`) +* **Bug** - Fixed `clone()` mangling typed arrays, `ArrayBuffer`, and `DataView` into index-keyed plain objects — these now clone into independent buffers of the correct type ### Component * **Feature** - All callbacks now receive a `rerender()` function to fully rerender the DOM of the component. diff --git a/packages/utils/bench/cloning.bench.js b/packages/utils/bench/cloning.bench.js index 300b2d3a9..c2e5c32fd 100644 --- a/packages/utils/bench/cloning.bench.js +++ b/packages/utils/bench/cloning.bench.js @@ -58,21 +58,90 @@ const mixedTypes = { map: new Map([['key', 'value']]), }; +// Deep tree: boundary-call cost of structuredClone is roughly fixed per call, +// so a large structure is where it can amortize that overhead against the +// hand-rolled recursive walk. Built from plain objects/arrays only so both +// paths produce equivalent results. +const deepTree = (() => { + const make = (depth) => { + if (depth === 0) { return { leaf: true, n: depth, tag: 'end' }; } + return { + depth, + label: `node-${depth}`, + children: [make(depth - 1), make(depth - 1)], + values: [depth, depth * 2, depth * 3], + }; + }; + return make(10); +})(); + +// TypedArray-bearing shape. NOTE: clone() does not currently preserve typed +// arrays (they fall through to the plain-object branch and get mangled into +// index-keyed objects), so this pair is a correctness contrast, not a like-for- +// like speed comparison. structuredClone is the only correct option here. +const typedData = { + weights: new Float64Array(1024), + ids: new Int32Array(1024), + label: 'layer-weights', +}; + /******************************* Benchmarks *******************************/ -describe('clone', () => { - bench('flat 8-key object', () => { +// Grouped by shape so vitest's comparative reporter shows clone vs +// structuredClone relative within each block. + +describe('flat 8-key object', () => { + bench('clone', () => { clone(flatSettings); }); - bench('20-key wide object', () => { + bench('structuredClone', () => { + structuredClone(flatSettings); + }); +}); + +describe('20-key wide object', () => { + bench('clone', () => { clone(wideSettings); }); - bench('nested state with arrays', () => { + bench('structuredClone', () => { + structuredClone(wideSettings); + }); +}); + +describe('nested state with arrays', () => { + bench('clone', () => { clone(nestedState); }); - bench('mixed types (Date, RegExp, Set, Map)', () => { + bench('structuredClone', () => { + structuredClone(nestedState); + }); +}); + +describe('mixed types (Date, RegExp, Set, Map)', () => { + bench('clone', () => { clone(mixedTypes); }); + bench('structuredClone', () => { + structuredClone(mixedTypes); + }); +}); + +describe('deep tree (depth 10)', () => { + bench('clone', () => { + clone(deepTree); + }); + bench('structuredClone', () => { + structuredClone(deepTree); + }); +}); + +describe('typed arrays (correctness contrast)', () => { + bench('clone', () => { + clone(typedData); + }); + bench('structuredClone', () => { + structuredClone(typedData); + }); }); diff --git a/packages/utils/src/cloning.js b/packages/utils/src/cloning.js index a5fb9491f..5d87d14ed 100644 --- a/packages/utils/src/cloning.js +++ b/packages/utils/src/cloning.js @@ -1,4 +1,14 @@ -import { isArray, isClassInstance, isDate, isMap, isObject, isPlainObject, isRegExp, isSet } from './types.js'; +import { + isArray, + isBinary, + isClassInstance, + isDate, + isMap, + isObject, + isPlainObject, + isRegExp, + isSet, +} from './types.js'; /*------------------- Cloning @@ -88,6 +98,12 @@ const cloneValue = (src, preserveDOM, preserveNonCloneable, seen) => { copy.add(cloneValue(v, preserveDOM, preserveNonCloneable, seen)); } } + // the one case where the native clone wins: ~6x faster on binary data + // where the recursive walk only loses, see cloning.bench.js + else if (isBinary(src)) { + copy = structuredClone(src); + seen.set(src, copy); + } else if (isObject(src)) { if (preserveNonCloneable && isClassInstance(src)) { return src; diff --git a/packages/utils/test/cloning.test.js b/packages/utils/test/cloning.test.js index 8fd5dd2f7..87eb120a2 100644 --- a/packages/utils/test/cloning.test.js +++ b/packages/utils/test/cloning.test.js @@ -55,6 +55,42 @@ describe('clone', () => { expect(clonedMap).not.toBe(originalMap); // Ensure it's a deep clone }); + it('should clone typed arrays into an independent buffer', () => { + const original = new Float64Array([1.5, 2.5, 3.5]); + const cloned = clone(original); + expect(cloned).toBeInstanceOf(Float64Array); + expect(cloned).not.toBe(original); + expect(Array.from(cloned)).toEqual([1.5, 2.5, 3.5]); + + // mutating the original must not bleed into the clone + original[0] = 99; + expect(cloned[0]).toBe(1.5); + }); + + it('should clone typed arrays nested in objects', () => { + const original = { weights: new Int32Array([10, 20, 30]), label: 'layer' }; + const cloned = clone(original); + expect(cloned.weights).toBeInstanceOf(Int32Array); + expect(cloned.weights).not.toBe(original.weights); + expect(Array.from(cloned.weights)).toEqual([10, 20, 30]); + }); + + it('should clone ArrayBuffer and DataView', () => { + const buffer = new ArrayBuffer(8); + new DataView(buffer).setFloat64(0, 3.14); + + const clonedBuffer = clone(buffer); + expect(clonedBuffer).toBeInstanceOf(ArrayBuffer); + expect(clonedBuffer).not.toBe(buffer); + expect(new DataView(clonedBuffer).getFloat64(0)).toBe(3.14); + + const view = new DataView(buffer); + const clonedView = clone(view); + expect(clonedView).toBeInstanceOf(DataView); + expect(clonedView).not.toBe(view); + expect(clonedView.getFloat64(0)).toBe(3.14); + }); + it('should clone deep objects', () => { const originalObject = { level1: { From 1e4295ac8180fc9fa4a7d54a3d5597b33a7e5318 Mon Sep 17 00:00:00 2001 From: jlukic Date: Wed, 20 May 2026 11:10:19 -0400 Subject: [PATCH 17/19] Bug/Perf: Fix binary data in clone() --- packages/utils/src/cloning.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/utils/src/cloning.js b/packages/utils/src/cloning.js index 5d87d14ed..cdc25bf6d 100644 --- a/packages/utils/src/cloning.js +++ b/packages/utils/src/cloning.js @@ -98,9 +98,9 @@ const cloneValue = (src, preserveDOM, preserveNonCloneable, seen) => { copy.add(cloneValue(v, preserveDOM, preserveNonCloneable, seen)); } } - // the one case where the native clone wins: ~6x faster on binary data - // where the recursive walk only loses, see cloning.bench.js else if (isBinary(src)) { + // structuredClone beats hand-rolled in perf for binary dramatically (6x) + // loses out to handrolled for other code-shapes see copy = structuredClone(src); seen.set(src, copy); } From 9f9a8730e769157df18b6a57b9f10fe8e5abbdc8 Mon Sep 17 00:00:00 2001 From: jlukic Date: Wed, 20 May 2026 11:19:55 -0400 Subject: [PATCH 18/19] Refactor: Isolate instance state in createReactiveState, share defaultState MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit defaultState is the definition's declaration and is correctly shared across instances (Template.clone manifests, it does not duplicate). The leak was that in reference mode each instance's Signal aliased that shared default and a mutation wrote through into the prototype. Isolate at the seam where instance state is derived from the defaults — clone the seeded object value per Signal — instead of cloning defaultState at clone(). Restores the shared-defaultState contract (subtemplate-settings Template.clone tests pass unchanged) and keeps the per-read reference win. --- packages/templating/src/template.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/templating/src/template.js b/packages/templating/src/template.js index 63dea4534..9321b0f48 100644 --- a/packages/templating/src/template.js +++ b/packages/templating/src/template.js @@ -144,7 +144,11 @@ export const Template = class Template { if (dataValue !== undefined) { return dataValue; } - return config?.value ?? config; + // reference-mode Signals alias their initial value; clone object defaults so an instance can't mutate the prototype's shared declaration + const defaultValue = config?.value ?? config; + return (defaultValue !== null && typeof defaultValue === 'object') + ? clone(defaultValue, { preserveNonCloneable: true }) + : defaultValue; }; each(defaultState, (config, name) => { @@ -434,8 +438,7 @@ export const Template = class Template { element: this.element, ast: this.ast, css: this.css, - // each template instance needs its own copy of state since signals may be pass-by-reference - defaultState: clone(this.defaultState, { preserveNonCloneable: true }), + defaultState: this.defaultState, defaultSettings: this.defaultSettings, events: this.events, keys: this.keys, From 4968ed390ddf7987957a4547d5561df84f1abf32 Mon Sep 17 00:00:00 2001 From: jlukic Date: Wed, 20 May 2026 11:21:37 -0400 Subject: [PATCH 19/19] Refactor: Use isObject helper for the default-value clone guard --- packages/templating/src/template.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/templating/src/template.js b/packages/templating/src/template.js index 9321b0f48..0437520e5 100644 --- a/packages/templating/src/template.js +++ b/packages/templating/src/template.js @@ -16,6 +16,7 @@ import { isClient, isEqual, isFunction, + isObject, isServer, kebabToCamel, mapObject, @@ -146,9 +147,7 @@ export const Template = class Template { } // reference-mode Signals alias their initial value; clone object defaults so an instance can't mutate the prototype's shared declaration const defaultValue = config?.value ?? config; - return (defaultValue !== null && typeof defaultValue === 'object') - ? clone(defaultValue, { preserveNonCloneable: true }) - : defaultValue; + return isObject(defaultValue) ? clone(defaultValue, { preserveNonCloneable: true }) : defaultValue; }; each(defaultState, (config, name) => {