Open
Conversation
Foundation for three-mode signal safety (freeze/reference/none). defaultSafety starts at 'freeze'; setDefaultSafety validates against the valid set. No consumers yet — the Signal class will wire up in a follow-up.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
Three-mode safety: 'freeze' (default), 'reference', 'none'. Reads return the protected ref directly; no more per-read cloning. Mutations on frozen values throw at the call site instead of being silently swallowed. - protect() gate: freeze → deepFreeze, otherwise pass-through - value getter/peek() return currentValue directly (no readCopy) - get() loses its options form — use .peek() or .clone() for escapes - new .clone() method returns a deep copy; tracked like .get() - in-place helpers (push/unshift/splice/setIndex/removeIndex/ setArrayProperty/setProperty) branch on safety so reference/none retain O(1) mutation; freeze path rebuilds via spread - mutate(fn): return-value path sets; undefined path notifies (in-place mutation only works under reference/none; throws on freeze) - legacy allowClone: false maps to 'reference' for back-compat Reactivity package tests pass. Downstream callsites that mutated through peek() or the data context will surface as loud failures under freeze; those need per-callsite fixes in follow-up commits.
Replace mutate-in-map with spread-in-map so enrichment helpers don't mutate the caller's input. Two callsites surfaced under freeze-on-set: - mobile-menu addNavIcons: was mutating item.navIcon on items from settings.menu, silently modifying the caller's menu tree - nav-menu addSelectedIndex: was mutating item.selectedIndex on items from settings.menu, same issue; inner .map passes now use the returned value instead of relying on side effects
jlukic
added a commit
that referenced
this pull request
Apr 15, 2026
PR #148 (signal-safety refactor under packages/reactivity) exposed the per-package scoping bug in discover. The prior logic looked up tachometer configs inside the specific changed package and ran their matrix cells — but every tachometer config in this repo lives under packages/renderer, so a PR that only changes packages/reactivity resolved to an empty matrix and the bench workflow skipped silently. This is wrong in the base case: reactivity imports renderer consumes via workspace symlinks, so a signal-path change moves renderer's bench numbers. Same for templating → rendering, utils → everything, etc. The benches are end-to-end measurements that pull the whole framework through a workload; treating them as scoped-by-touched-package was premature. New logic: any trigger that reaches this workflow (push to main, or a pull_request that matched our path filter) runs every tachometer-ci*.json it finds under packages/*/bench/tachometer/. Path filter already restricts triggers to perf-relevant changes, so this doesn't over-fire. The push-branch and pull-request branches collapse to the same one-liner, which is a good sign. Ship.
jlukic
added a commit
that referenced
this pull request
Apr 15, 2026
Build: Discover runs all benchmarkable packages, not just touched ones PR #148 (signal-safety refactor under packages/reactivity) exposed the per-package scoping bug in discover. The prior logic looked up tachometer configs inside the specific changed package and ran their matrix cells — but every tachometer config in this repo lives under packages/renderer, so a PR that only changes packages/reactivity resolved to an empty matrix and the bench workflow skipped silently. This is wrong in the base case: reactivity imports renderer consumes via workspace symlinks, so a signal-path change moves renderer's bench numbers. Same for templating → rendering, utils → everything, etc. The benches are end-to-end measurements that pull the whole framework through a workload; treating them as scoped-by-touched-package was premature. New logic: any trigger that reaches this workflow (push to main, or a pull_request that matched our path filter) runs every tachometer-ci*.json it finds under packages/*/bench/tachometer/. Path filter already restricts triggers to perf-relevant changes, so this doesn't over-fire. The push-branch and pull-request branches collapse to the same one-liner, which is a good sign. Ship.
Build: Discover runs all benchmarkable packages, not just touched ones PR #148 (signal-safety refactor under packages/reactivity) exposed the per-package scoping bug in discover. The prior logic looked up tachometer configs inside the specific changed package and ran their matrix cells — but every tachometer config in this repo lives under packages/renderer, so a PR that only changes packages/reactivity resolved to an empty matrix and the bench workflow skipped silently. This is wrong in the base case: reactivity imports renderer consumes via workspace symlinks, so a signal-path change moves renderer's bench numbers. Same for templating → rendering, utils → everything, etc. The benches are end-to-end measurements that pull the whole framework through a workload; treating them as scoped-by-touched-package was premature. New logic: any trigger that reaches this workflow (push to main, or a pull_request that matched our path filter) runs every tachometer-ci*.json it finds under packages/*/bench/tachometer/. Path filter already restricts triggers to perf-relevant changes, so this doesn't over-fire. The push-branch and pull-request branches collapse to the same one-liner, which is a good sign. Ship.
🟡 Mixed Performance (Net Positive) for
|
| metric | Improvement |
|---|---|
signal-reactive-set-index-300 |
-98% (102ms) 🏆 |
signal-reactive-list-filter-1000x300 |
-94% (107ms) 🏆 |
signal-reactive-set-property-by-id-200 |
-88% (181ms) 🏆 |
signal-reactive-push-2000x20 |
-82% (176ms) 🏆 |
signal-computed-chain-10x60k |
-9% (19ms) |
toggle-first-10 |
-6% (11ms) |
signal-reactive-multi-read-5x160k |
-5% (11ms) |
remove-last-10 |
-5% (7ms) |
toggle-middle-10 |
-4% (7ms) |
remove-first-10 |
-4% (7ms) |
❌ Slower (4)
Metrics where this PR confidently regressed performance compared to main.
| metric | Regression |
|---|---|
remove-5-front |
+7% (6ms) |
signal-reactive-list-replace-1000x1000 |
+5% (15ms) |
clear-completed-250 |
+4% (2ms) |
toggle-all-20 |
+4% (14ms) |
📜 Regressions from peak (17)
These metrics were better on a prior commit than they are now. The peak CI dominates current CI — not attributable to per-sample noise. Bisect candidates are the commits between the peak and HEAD; nearest-to-peak is usually the best bet.
⚪ No Change (13)
Metrics where this PR measured within ±2% of main — no meaningful performance change detected.
| metric | Change |
|---|---|
add-20 |
-0.1% – +0.1% |
create-10k |
-0.5% – +0.8% |
create-1k |
-1.5% – +0.3% |
edit-cycle-5 |
+0.7% – +1.6% |
filter-cycle-20 |
-1.7% – -0.5% |
remove-middle-10 |
+0.1% – +0.6% |
remove-row-back-10 |
-0.5% – +1.0% |
remove-row-front-20 |
-1.8% – -0.6% |
replace-1k |
-1.6% – +0.3% |
swap-rows-20 |
-0.9% – +0.3% |
toggle-10 |
+0.2% – +0.9% |
toggle-last-10 |
-1.4% – +0.2% |
update-10th-10 |
-1.6% – +0.9% |
🔍 Unsure (9)
Inconclusive (8)
The measured difference is small, and our sampling couldn't confidently place it above or below zero. Running more samples in a future run might settle these metrics.
| metric | Change | Expected Noise |
|---|---|---|
append-1k |
+0.4% – +5.8% | ±1% |
bulk-add-500 |
-4.1% – -2.0% | ±1% |
clear-10k |
-4.7% – +1.8% | ±1% |
edit-start-10 |
-2.5% – -0.3% | ±1% |
remove-5-back |
-0.9% – +3.8% | ±2% |
remove-row-middle-20 |
-2.1% – -0.3% | ±0% |
select-40 |
-2.2% – -0.8% | ±0% |
signal-reactive-fanout-500x1200 |
-1.3% – +2.1% | ±2% |
Too Fast to Measure Precisely (1)
On benches this short, system jitter (scheduling, GC, JIT) masks sub-4% changes; larger deltas still resolve cleanly.
| metric | Change | Test Time | Expected Noise |
|---|---|---|---|
remove-10-middle |
-3.4% – -1.7% | ~156ms | ±1% |
Sample size: 50 · Resolution floor: ±2% · Timeout: 3min · Wall-clock: 16m13s
Surfaces when a signal-backed data blob flows into the templating
boundary under freeze-default:
- extend was copying source property descriptors verbatim, so a frozen
source produced a fresh target with {writable: false, configurable:
false} on every property. Target was extensible but key-level writes
threw. Now extend installs a fresh writable descriptor for data
properties and only preserves descriptors for accessor (getter/setter)
properties.
- template.setDataContext thaws this.data lazily when it arrives frozen
(via a Template clone with the frozen data argument); assignInPlace
then mutates in place as before, preserving reference identity and
key-level diffing semantics that the spurious-reactivity guards
depend on.
- ssr-hydration test updated to clone the array before mutating (the
old pattern silently mutated the peek() return).
…tic-Next into perf/signal-safety-v2
github-actions
Bot
added
Templating
Static surface for configuring defaults, discoverable via Signal.* tab
completion and validated on assignment:
- Signal.equalityFunction / Signal.cloneFunction — accessors with
typeof check; throw TypeError on non-function
- Signal.safety / Signal.tracing / Signal.stackCapture — accessors
delegating to helpers module-level state
- Signal.configure({...}) — bulk wrapper; forwards each key through
its accessor so validation runs
- Signal.defaults — snapshot getter of current settings
Drop the allowClone option. The transitional mapping from
allowClone: false → safety: 'reference' is removed. Callsites migrated:
- lit/renderer.js dataVersion — collapses cleanly to safety: 'none'
(allowClone:false + equalityFunction always-false)
- native/blocks/each.js itemSignal — safety: 'reference'
- templating/src/template.js settingsVars — safety: 'reference'
- bench.js / bench-todo.js / tools/benchmark — safety: 'none'
Drop legacy function-form statics (Signal.setTracing, Signal.isTracing,
Signal.setStackCapture, Signal.isStackCapture). The tracing flag
feature was never released; no compat needed.
noop becomes a true no-op — returns undefined, swallows arguments. identity is the new export for the "return first argument unchanged" semantic the old noop was secretly providing. Callsites that depended on the identity behavior migrated to identity: - utils/src/strings.js joinWords transform — the return value feeds the quoting path - component/define-component.js createComponent — return becomes the instance methods (and when no createComponent is provided, the identity default carries callParams into the instance, which the SSR env-guard path depends on) - query/behavior.js createBehavior — same shape as createComponent - query/behavior.js customInvocation — fallback method-not-found handler; return value is the found result All other `= noop` callsites are lifecycle side-effect callbacks (onCreated, onRendered, etc.) whose return values are discarded — true noop is correct there. CHANGELOG entries added for the Reactivity + Utils breaking changes that landed in this release.
github-actions
Bot
added
Query
…aths - template.js setDataContext: remove Object.isExtensible check. Ran 200-300x per each-block reconcile on subtemplate renders. Local 4-sample A/B measurements show removing it eliminates the remove-middle +11.7% and remove-last +13% regressions identified from aggregated CI artifacts. - signal.js: revert inlined protect() in value setter (speculative, not diagnostic-backed). Keep protect() as a method. - signal.js: revert setArrayProperty single-index fast-path (speculative). - bench-todo.js: scale short benches (toggle/edit/remove single-ops) 10x so they run 120-360ms instead of 6-20ms. Short benches were in tachometer's noise band and rotated between confident-regressed and unsure across runs. With the default safety now 'reference', no signal values are frozen under default use, so template.data will be mutable from the start and setDataContext never needs to detect+replace a frozen target. Users who opt into safety='freeze' can still hit this edge case — if that surfaces, fix it at the signal boundary (don't pass frozen values to Template constructor) rather than defensive checks in a per-call path.
…xtend docstring
hasProperty becomes a thin re-export of Object.hasOwn (ES2022); proxyObject default switches from noop to an explicit () => ({}); extend gets a docstring capturing its modern-JS shallow-merge semantics.
Shared mutable state now lives on a single config object in helpers.js; Signal's tracing/safety/stackCapture accessors touch it directly instead of routing through setter helpers. Housekeeping statics moved to the bottom of signal.js so the constructor and happy path read at the top, and the mutate() reference/none dedupe path restores its snapshot+equality guard.
Captures decisions for the next utils release window (returnsSelf/returnsTrue/etc. family, remove `any` alias, fix first/last return-type polymorphism). Agent lessons gains "Quiet Code Over Ornamented Code" warning against SCREAMING_CAPS, underscore-prefixes, and dispatcher-layer patterns.
The test enumerated packages/ raw, so stray harness dirs like .claude/ broke the dist/cdn/ existence check.
Bracket assignment already routes through the safety/tracing/stackCapture setters, so the explicit if-ladder was ornament. Agent lessons updated to frame the quiet-code check as a changeset review pass, not just a write-time discipline.
Drops session-scaffolding narration from signal.js, helpers.js, and siblings; keeps load-bearing WHY notes (WeakRef self-stop, captureStackTrace cost, mutate behavior by safety preset). Signal class reorganized with section dividers (Core / Complex / Mutation Helpers / Tracing / Instance of / Configuration), signalTag moved to helpers.js, scheduler's flushTask wrapper replaced by direct Scheduler.flush reference.
Captures the bar for shipped comments (non-obvious to someone who doesn't know the codebase, or a weird trick) with an IS_SIGNAL before/after example, and clarifies that level-2 section dividers for multi-method conceptual clusters are distinct from single-declaration narration labels.
jlukic
changed the title
jlukic
added a commit
that referenced
this pull request
Apr 18, 2026
Walk prior bench runs on this PR's branch to build a per-iteration history. The reporter merges it with bench-history.json (main commits) so peak attribution spans BOTH main AND this PR's iterations. An agent iterating on a perf branch now sees: "create-1k peaked at 104.7ms on commit 9bcd3f1 (iteration 10), now at 143.9ms (+37%). Bisect candidates: 442446e, 64c6820, 5595cbd +7 more." New files: - tools/bench-reporter/fetch-pr-history.js — walks gh API for prior successful Benchmarks runs on the PR branch, downloads results-* artifacts, extracts this-change absolute CIs, outputs pr-history.json in bench-history.json schema. Changed files: - reporter.js — new --pr-history flag, mergeHistories() combines main + PR iteration histories sorted by timestamp with SHA dedup. The existing computeHistoryStatus + REOPENED rendering work unchanged. - benchmarks-report.yml — new "Fetch PR iteration history" step before "Generate report" in the comment job. Uses the bot token for API auth. Tested against PR #148's 17 prior runs: 13 REOPENED metrics surfaced with correct peak attribution and bisect candidates.
jlukic
added a commit
that referenced
this pull request
Apr 18, 2026
Walk prior bench runs on this PR's branch to build a per-iteration history. The reporter merges it with bench-history.json (main commits) so peak attribution spans BOTH main AND this PR's iterations. An agent iterating on a perf branch now sees: "create-1k peaked at 104.7ms on commit 9bcd3f1 (iteration 10), now at 143.9ms (+37%). Bisect candidates: 442446e, 64c6820, 5595cbd +7 more." New files: - tools/bench-reporter/fetch-pr-history.js — walks gh API for prior successful Benchmarks runs on the PR branch, downloads results-* artifacts, extracts this-change absolute CIs, outputs pr-history.json in bench-history.json schema. Changed files: - reporter.js — new --pr-history flag, mergeHistories() combines main + PR iteration histories sorted by timestamp with SHA dedup. The existing computeHistoryStatus + REOPENED rendering work unchanged. - benchmarks-report.yml — new "Fetch PR iteration history" step before "Generate report" in the comment job. Uses the bot token for API auth. Tested against PR #148's 17 prior runs: 13 REOPENED metrics surfaced with correct peak attribution and bisect candidates.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds
safetyparam to signals, currently defaults toreference