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/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 diff --git a/ai/skills/contributing/investigate-performance.md b/ai/skills/contributing/investigate-performance.md new file mode 100644 index 000000000..00414c515 --- /dev/null +++ b/ai/skills/contributing/investigate-performance.md @@ -0,0 +1,454 @@ +--- +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. + +**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. + +**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. + +**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 + +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. + +❌ "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. + +--- + +## 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 weighted targets, clear the weight gate → /read-bench-report +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 + → 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 +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. + +**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. + +**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 + +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` | 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×. + +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. + +**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). 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 +- `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. 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. + +**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 + +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 + +**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. + +--- + +## Step 4 — Gather: Capture Chrome DevTools Traces + +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:** + +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 + # flip the single source line under test, rebuild baseline, then restore it + node build-ci.js baseline + ``` + + 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 & + ``` + +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 current-self vs baseline-self per function — biggest positive Δ is your suspect +``` + +**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 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. + +❌ 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) +- "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. + +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 + +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 6 — Local Tachometer: Reproduce, Size, and A/B + +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. + +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. 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: +/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 /packages//bench/tachometer || exit 1 +exec /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 7 — Read the Suspect Framework Source + +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: + +- 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 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. + +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. 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. + +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. + +--- + +## 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. + +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 + +**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) + +**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 + +**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 + +**"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 + +**"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 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.) + +--- + +## Quick Reference + +| Phase | Tool | Output | +|---|---|---| +| 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 | +| 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 | + +--- + +## 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 | +| **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 | 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..111a72890 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 = 'reference'; + + 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..86ab6dc33 100644 --- a/packages/reactivity/test/unit/internals.test.js +++ b/packages/reactivity/test/unit/internals.test.js @@ -765,24 +765,37 @@ 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', () => { + 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/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..ac68a1deb 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', () => { @@ -1000,17 +701,26 @@ 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); }); - 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); }); @@ -1037,16 +747,16 @@ 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); }); }); /*********************************************** - * get() / value getter — reactive read with cloning + * get() / value getter ***********************************************/ describe('get() and value', () => { @@ -1061,14 +771,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()); @@ -1152,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); @@ -1161,7 +863,7 @@ describe('Signal API', () => { }); /*********************************************** - * subscribe() — callback observer that fires once synchronously + on changes + * subscribe() — callback observer ***********************************************/ describe('subscribe()', () => { @@ -1203,44 +905,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 +913,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 +987,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 +1085,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/blocks/each.js b/packages/renderer/src/engines/native/blocks/each.js index e0ac2702c..65fa55d2a 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); @@ -609,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 { 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/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'); }, }), }); 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..0437520e5 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, @@ -15,6 +16,7 @@ import { isClient, isEqual, isFunction, + isObject, isServer, kebabToCamel, mapObject, @@ -143,7 +145,9 @@ 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 isObject(defaultValue) ? clone(defaultValue, { preserveNonCloneable: true }) : defaultValue; }; each(defaultState, (config, name) => { @@ -1001,7 +1005,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 +1023,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/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..cdc25bf6d 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)); } } + 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); + } else if (isObject(src)) { if (preserveNonCloneable && isClassInstance(src)) { return src; 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/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: { 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 }) => ({