fix: improve indexed each array reconcilation (#10422)

* fix: improve indexed each array reconcilation

* simplify
pull/10430/head
Dominic Gannaway 9 months ago committed by GitHub
parent 623340a1de
commit 0e011add4e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -0,0 +1,5 @@
---
"svelte": patch
---
fix: improve indexed each array reconcilation

@ -14,14 +14,11 @@ import {
set_current_hydration_fragment set_current_hydration_fragment
} from './hydration.js'; } from './hydration.js';
import { clear_text_content, map_get, map_set } from './operations.js'; import { clear_text_content, map_get, map_set } from './operations.js';
import { STATE_SYMBOL } from './proxy.js';
import { insert, remove } from './reconciler.js'; import { insert, remove } from './reconciler.js';
import { empty } from './render.js'; import { empty } from './render.js';
import { import {
destroy_signal, destroy_signal,
execute_effect, execute_effect,
is_lazy_property,
lazy_property,
mutable_source, mutable_source,
push_destroy_fn, push_destroy_fn,
render_effect, render_effect,
@ -132,7 +129,7 @@ function each(anchor_node, collection, flags, key_fn, render_fn, fallback_fn, re
}; };
/** @param {import('./types.js').EachBlock} block */ /** @param {import('./types.js').EachBlock} block */
const clear_each = (block) => { const render_each = (block) => {
const flags = block.f; const flags = block.f;
const is_controlled = (flags & EACH_IS_CONTROLLED) !== 0; const is_controlled = (flags & EACH_IS_CONTROLLED) !== 0;
const anchor_node = block.a; const anchor_node = block.a;
@ -175,7 +172,7 @@ function each(anchor_node, collection, flags, key_fn, render_fn, fallback_fn, re
if (fallback_fn !== null) { if (fallback_fn !== null) {
if (length === 0) { if (length === 0) {
if (block.v.length !== 0 || render === null) { if (block.v.length !== 0 || render === null) {
clear_each(block); render_each(block);
create_fallback_effect(); create_fallback_effect();
return; return;
} }
@ -201,7 +198,7 @@ function each(anchor_node, collection, flags, key_fn, render_fn, fallback_fn, re
false false
); );
render = render_effect(clear_each, block, true); render = render_effect(render_each, block, true);
if (mismatch) { if (mismatch) {
// Set a fragment so that Svelte continues to operate in hydration mode // Set a fragment so that Svelte continues to operate in hydration mode
@ -279,14 +276,9 @@ function reconcile_indexed_array(
flags, flags,
apply_transitions apply_transitions
) { ) {
var is_proxied_array = STATE_SYMBOL in array && /** @type {any} */ (array[STATE_SYMBOL]).i;
var a_blocks = each_block.v; var a_blocks = each_block.v;
var active_transitions = each_block.s; var active_transitions = each_block.s;
if (is_proxied_array) {
flags &= ~EACH_ITEM_REACTIVE;
}
/** @type {number | void} */ /** @type {number | void} */
var a = a_blocks.length; var a = a_blocks.length;
@ -334,7 +326,7 @@ function reconcile_indexed_array(
break; break;
} }
item = is_proxied_array ? lazy_property(array, index) : array[index]; item = array[index];
block = each_item_block(item, null, index, render_fn, flags); block = each_item_block(item, null, index, render_fn, flags);
b_blocks[index] = block; b_blocks[index] = block;
@ -349,7 +341,7 @@ function reconcile_indexed_array(
for (; index < length; index++) { for (; index < length; index++) {
if (index >= a) { if (index >= a) {
// Add block // Add block
item = is_proxied_array ? lazy_property(array, index) : array[index]; item = array[index];
block = each_item_block(item, null, index, render_fn, flags); block = each_item_block(item, null, index, render_fn, flags);
b_blocks[index] = block; b_blocks[index] = block;
insert_each_item_block(block, dom, is_controlled, null); insert_each_item_block(block, dom, is_controlled, null);
@ -789,17 +781,6 @@ function update_each_item_block(block, item, index, type) {
const block_v = block.v; const block_v = block.v;
if ((type & EACH_ITEM_REACTIVE) !== 0) { if ((type & EACH_ITEM_REACTIVE) !== 0) {
set_signal_value(block_v, item); set_signal_value(block_v, item);
} else if (is_lazy_property(block_v)) {
// If we have lazy properties, it means that an array was used that has been
// proxied. Given this, we need to re-sync the old array by mutating the backing
// value to be the latest value to ensure the UI updates correctly. TODO: maybe
// we should bypass any internal mutation checks for this?
const o = block_v.o;
const p = block_v.p;
const prev = o[p];
if (prev !== item) {
o[p] = item;
}
} }
const transitions = block.s; const transitions = block.s;
const index_is_reactive = (type & EACH_INDEX_REACTIVE) !== 0; const index_is_reactive = (type & EACH_INDEX_REACTIVE) !== 0;
@ -856,9 +837,7 @@ export function destroy_each_item_block(
/** /**
* @template V * @template V
* @template O * @param {V} item
* @template P
* @param {V | import('./types.js').LazyProperty<O, P>} item
* @param {unknown} key * @param {unknown} key
* @param {number} index * @param {number} index
* @param {(anchor: null, item: V, index: number | import('./types.js').Signal<number>) => void} render_fn * @param {(anchor: null, item: V, index: number | import('./types.js').Signal<number>) => void} render_fn

@ -39,7 +39,6 @@ const FLUSH_MICROTASK = 0;
const FLUSH_SYNC = 1; const FLUSH_SYNC = 1;
export const UNINITIALIZED = Symbol(); export const UNINITIALIZED = Symbol();
export const LAZY_PROPERTY = Symbol();
// Used for controlling the flush of effects. // Used for controlling the flush of effects.
let current_scheduler_mode = FLUSH_MICROTASK; let current_scheduler_mode = FLUSH_MICROTASK;
@ -1542,20 +1541,6 @@ export function is_signal(val) {
); );
} }
/**
* @template O
* @template P
* @param {any} val
* @returns {val is import('./types.js').LazyProperty<O, P>}
*/
export function is_lazy_property(val) {
return (
typeof val === 'object' &&
val !== null &&
/** @type {import('./types.js').LazyProperty<O, P>} */ (val).t === LAZY_PROPERTY
);
}
/** /**
* @template V * @template V
* @param {unknown} val * @param {unknown} val
@ -2084,21 +2069,6 @@ export function inspect(get_value, inspect = console.log) {
}); });
} }
/**
* @template O
* @template P
* @param {O} o
* @param {P} p
* @returns {import('./types.js').LazyProperty<O, P>}
*/
export function lazy_property(o, p) {
return {
o,
p,
t: LAZY_PROPERTY
};
}
/** /**
* @template V * @template V
* @param {V} value * @param {V} value
@ -2109,9 +2079,6 @@ export function unwrap(value) {
// @ts-ignore // @ts-ignore
return get(value); return get(value);
} }
if (is_lazy_property(value)) {
return value.o[value.p];
}
// @ts-ignore // @ts-ignore
return value; return value;
} }

@ -11,7 +11,7 @@ import {
SNIPPET_BLOCK SNIPPET_BLOCK
} from './block.js'; } from './block.js';
import type { READONLY_SYMBOL, STATE_SYMBOL } from './proxy.js'; import type { READONLY_SYMBOL, STATE_SYMBOL } from './proxy.js';
import { DERIVED, EFFECT, RENDER_EFFECT, SOURCE, PRE_EFFECT, LAZY_PROPERTY } from './runtime.js'; import { DERIVED, EFFECT, RENDER_EFFECT, SOURCE, PRE_EFFECT } from './runtime.js';
// Put all internal types in this file. Once we convert to JSDoc, we can make this a d.ts file // Put all internal types in this file. Once we convert to JSDoc, we can make this a d.ts file
@ -123,12 +123,6 @@ export type MaybeSignal<T = unknown> = T | Signal<T>;
export type UnwrappedSignal<T> = T extends Signal<infer U> ? U : T; export type UnwrappedSignal<T> = T extends Signal<infer U> ? U : T;
export type LazyProperty<O, P> = {
o: O;
p: P;
t: typeof LAZY_PROPERTY;
};
export type EqualsFunctions<T = any> = (a: T, v: T) => boolean; export type EqualsFunctions<T = any> = (a: T, v: T) => boolean;
export type BlockType = export type BlockType =

@ -0,0 +1,43 @@
import { flushSync } from 'svelte';
import { test } from '../../test';
export default test({
html: `<ul><li><button>Delete</button>\na\na</li><li><button>Delete</button>\nb\nb</li><li><button>Delete</button>\nc\nc</li><li><button>Delete</button>\nd\nd</li></ul>`,
async test({ assert, target }) {
/**
* @type {{ click: () => void; }}
*/
let btn1;
[btn1] = target.querySelectorAll('button');
flushSync(() => {
btn1.click();
});
assert.htmlEqual(
target.innerHTML,
`<ul><li><button>Delete</button>\nb\nb</li><li><button>Delete</button>\nc\nc</li><li><button>Delete</button>\nd\nd</li></ul>`
);
[btn1] = target.querySelectorAll('button');
flushSync(() => {
btn1.click();
});
assert.htmlEqual(
target.innerHTML,
`<ul><li><button>Delete</button>\nc\nc</li><li><button>Delete</button>\nd\nd</li></ul>`
);
[btn1] = target.querySelectorAll('button');
flushSync(() => {
btn1.click();
});
assert.htmlEqual(target.innerHTML, `<ul><li><button>Delete</button>\nd\nd</li></ul>`);
}
});

@ -0,0 +1,36 @@
<script>
let entries = $state([
{
id: 'a',
subitems: ['a'],
},
{
id: 'b',
subitems: ['b'],
},
{
id: 'c',
subitems: ['c'],
},
{
id: 'd',
subitems: ['d'],
},
]);
function onDeleteEntry(entry) {
entries = entries.filter((innerEntry) => innerEntry.id !== entry.id);
}
</script>
<ul>
{#each entries as entry}
<li>
<button on:click={() => onDeleteEntry(entry)}>Delete</button>
{entry.id}
{#each entry.subitems as subitem}
{subitem}
{/each}
</li>
{/each}
</ul>
Loading…
Cancel
Save