Conversation
The reconciler effect inside forEach (and the function-child effect in appendChild) runs deferred via the flush queue. When it ran, activeScope was typically undefined, so scope(...) calls inside them created scopes with no parent — unreachable from the surrounding render scope. Consequence: render() dispose did not tear down per-item reactive children. They kept firing on shared-signal changes and held references that prevented GC (~660 KB per 200x50 mount/dispose cycle leaked). Fix: capture activeScope at the synchronous setup site (forEach() call, appendChild function-child branch) and pass it explicitly as the parent of the inner scope(). Added regression test 14 in for-each.spec.mjs that uses a manual flush queue to reproduce the deferred-execution conditions (the rest of the suite uses a synchronous scheduler, under which the bug doesn't show).
The 'if (k === "children") continue' branch was unreachable under the
automatic JSX transform (jsx-runtime strips children from props before
delegating to createElement) and under normal classic JSX (children are
passed as varargs).
The only reachable callers were:
- Hand-written createElement({children: ...}) calls (tests only).
- The corner-case <div children="..." /> JSX attribute spelling, which
nobody uses in practice.
Removed both the guard and the one test that exercised it via a direct
createElement call. The other 'children' test still passes because it
uses jsx(), which strips children before calling createElement.
for...in on null/undefined is a spec'd no-op (skipped without iteration), so the guard added no correctness — only a branch. Micro-bench shows the guard-less version is marginally faster (~10% on the null-props path) and the V8 deopt profile is unchanged (same two training deopts on the for-in / inner call sites in both versions).
Spread of null/undefined is a no-op per spec ({...null} === {}), so the
?? {} fallback inside { ...(props ?? {}), children } is redundant.
Kept the other occurrence (compProps = props ?? {}) which is observable:
it provides a safe object argument to user components when called as
classic JSX <MyComp /> compiles to createElement(MyComp, null).
…entArray Both jsx() and jsxs() (and jsxDEV) now share a single function body that calls a new internal createElementArray(type, props, children) entry point. This skips the array -> spread(...) -> rest-array roundtrip that previously went through createElement's varargs signature. createElement remains a thin varargs wrapper that delegates to createElementArray, so the classic JSX transform and direct user calls are unchanged. Tight-loop bench (50K iter * 25 samples * 5-iter warmup, p25/p75 spread < 3%) shows a consistent 1-5% reduction on small-tree shapes: jsx <div/> 410 -> 391 ns (-5%) jsx <div>x</div> 917 -> 890 ns (-3%) jsx <div><a/></div> 1374 -> 1336 ns (-3%) jsxs 2 children 2230 -> 2180 ns (-2%) jsxs 3 children 4224 -> 4124 ns (-2%) jsxs 5 children 6428 -> 6345 ns (-1%) jsxs 1000 children 3561 -> 3561 us (DOM-bound, no change) Unifying jsx/jsxs/jsxDEV into one shared function body avoids inline- cache fragmentation that an earlier split-by-arity attempt exhibited.
The two comment markers `start` and `end` are always inserted into the same parent and never moved or removed during the function-child effect, so the sibling walk from `start.nextSibling` is guaranteed to reach `end` before running off the end of the sibling chain. The `nextSibling !== null` check was therefore redundant. Real-Chromium microbench (31 samples, per-op GC, N=0..100 children between markers) shows the simpler loop within noise of the guarded loop at every N — no perf regression, just less code.
|
createElementArray now passes { is } to document.createElement for host tags so <button is="my-tag"> produced by the runtime upgrades. SVG/createElementNS path untouched. Adds feature-guarded is.spec.mjs.
…Element createCustomElement builds an unregistered class (custom/scoped registries, manual define). defineElement is a thin autonomous-element facade (no extendElement). defineBuiltinElement handles customized built-ins. Adds tests for unregistered class, scoped registry isolation, and end-to-end built-in upgrade via the jsx is= runtime.
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.
No description provided.