diff --git a/.changeset/sharp-snakes-poke.md b/.changeset/sharp-snakes-poke.md new file mode 100644 index 000000000000..7f7f8aa7b2f9 --- /dev/null +++ b/.changeset/sharp-snakes-poke.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: emit `each_key_duplicate` error in production diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/EachBlock.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/EachBlock.js index 225a4f617c50..39f61272fcb9 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/EachBlock.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/EachBlock.js @@ -337,10 +337,6 @@ export function EachBlock(node, context) { const statements = [add_svelte_meta(b.call('$.each', ...args), node, 'each')]; - if (dev && node.metadata.keyed) { - statements.unshift(b.stmt(b.call('$.validate_each_keys', thunk, key_function))); - } - if (has_await) { context.state.init.push( b.stmt( diff --git a/packages/svelte/src/internal/client/dom/blocks/each.js b/packages/svelte/src/internal/client/dom/blocks/each.js index 006bf09257d1..c6f485c9d3fb 100644 --- a/packages/svelte/src/internal/client/dom/blocks/each.js +++ b/packages/svelte/src/internal/client/dom/blocks/each.js @@ -42,6 +42,8 @@ import { active_effect, get } from '../../runtime.js'; import { DEV } from 'esm-env'; import { derived_safe_equal } from '../../reactivity/deriveds.js'; import { current_batch } from '../../reactivity/batch.js'; +import { each_key_duplicate } from '../../errors.js'; +import { validate_each_keys } from '../../validate.js'; /** * The row of a keyed each block that is currently updating. We track this @@ -201,6 +203,11 @@ export function each(node, flags, get_collection, get_key, render_fn, fallback_f } was_empty = length === 0; + // skip if #each block isn't keyed + if (DEV && get_key !== index) { + validate_each_keys(array, get_key); + } + /** `true` if there was a hydration mismatch. Needs to be a `let` or else it isn't treeshaken out */ let mismatch = false; @@ -266,6 +273,8 @@ export function each(node, flags, get_collection, get_key, render_fn, fallback_f if (hydrating) { if (length === 0 && fallback_fn) { fallback = branch(() => fallback_fn(anchor)); + } else if (length > state.items.size) { + each_key_duplicate('', '', ''); } } else { if (should_defer_append()) { @@ -363,6 +372,7 @@ function reconcile( var is_animated = (flags & EACH_IS_ANIMATED) !== 0; var should_update = (flags & (EACH_ITEM_REACTIVE | EACH_INDEX_REACTIVE)) !== 0; + var count = 0; var length = array.length; var items = state.items; var first = state.first; @@ -451,6 +461,7 @@ function reconcile( stashed = []; current = prev.next; + count += 1; continue; } @@ -473,6 +484,12 @@ function reconcile( var start = stashed[0]; var j; + // full key uniqueness check is dev-only, + // key duplicates cause crash only due to `matched` being empty + if (matched.length === 0) { + each_key_duplicate('', '', ''); + } + prev = start.prev; var a = matched[0]; @@ -506,6 +523,7 @@ function reconcile( link(state, prev, item); prev = item; + count += 1; } continue; @@ -534,6 +552,14 @@ function reconcile( matched.push(item); prev = item; current = item.next; + count += 1; + } + + if (count !== length) { + // full key uniqueness check is dev-only, + // if keys duplication didn't cause a crash, + // the rendered list will be shorter then the array + each_key_duplicate('', '', ''); } if (current !== null || seen !== undefined) { diff --git a/packages/svelte/src/internal/client/validate.js b/packages/svelte/src/internal/client/validate.js index ec3d80544787..7c8cf93fdce3 100644 --- a/packages/svelte/src/internal/client/validate.js +++ b/packages/svelte/src/internal/client/validate.js @@ -7,35 +7,27 @@ import * as w from './warnings.js'; import { capture_store_binding } from './reactivity/store.js'; /** - * @param {() => any} collection + * @param {Array} array * @param {(item: any, index: number) => string} key_fn * @returns {void} */ -export function validate_each_keys(collection, key_fn) { - render_effect(() => { - const keys = new Map(); - const maybe_array = collection(); - const array = is_array(maybe_array) - ? maybe_array - : maybe_array == null - ? [] - : Array.from(maybe_array); - const length = array.length; - for (let i = 0; i < length; i++) { - const key = key_fn(array[i], i); - if (keys.has(key)) { - const a = String(keys.get(key)); - const b = String(i); +export function validate_each_keys(array, key_fn) { + const keys = new Map(); + const length = array.length; + for (let i = 0; i < length; i++) { + const key = key_fn(array[i], i); + if (keys.has(key)) { + const a = String(keys.get(key)); + const b = String(i); - /** @type {string | null} */ - let k = String(key); - if (k.startsWith('[object ')) k = null; + /** @type {string | null} */ + let k = String(key); + if (k.startsWith('[object ')) k = null; - e.each_key_duplicate(a, b, k); - } - keys.set(key, i); + e.each_key_duplicate(a, b, k); } - }); + keys.set(key, i); + } } /** diff --git a/packages/svelte/tests/runtime-legacy/samples/keyed-each-prod-unique-2/_config.js b/packages/svelte/tests/runtime-legacy/samples/keyed-each-prod-unique-2/_config.js new file mode 100644 index 000000000000..4caa45fd3cf2 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/keyed-each-prod-unique-2/_config.js @@ -0,0 +1,15 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + compileOptions: { + dev: false + }, + test({ assert, target }) { + let button = target.querySelector('button'); + + button?.click(); + + assert.throws(flushSync, /each_key_duplicate/); + } +}); diff --git a/packages/svelte/tests/runtime-legacy/samples/keyed-each-prod-unique-2/main.svelte b/packages/svelte/tests/runtime-legacy/samples/keyed-each-prod-unique-2/main.svelte new file mode 100644 index 000000000000..aedec8c57cba --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/keyed-each-prod-unique-2/main.svelte @@ -0,0 +1,8 @@ + + + +{#each data as d (d)} + {d} +{/each} diff --git a/packages/svelte/tests/runtime-legacy/samples/keyed-each-prod-unique-3/_config.js b/packages/svelte/tests/runtime-legacy/samples/keyed-each-prod-unique-3/_config.js new file mode 100644 index 000000000000..1f2add2a743c --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/keyed-each-prod-unique-3/_config.js @@ -0,0 +1,8 @@ +import { test } from '../../test'; + +export default test({ + compileOptions: { + dev: false + }, + error: 'each_key_duplicate\nKeyed each block has duplicate key at indexes and ' +}); diff --git a/packages/svelte/tests/runtime-legacy/samples/keyed-each-prod-unique-3/main.svelte b/packages/svelte/tests/runtime-legacy/samples/keyed-each-prod-unique-3/main.svelte new file mode 100644 index 000000000000..a05781bcb950 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/keyed-each-prod-unique-3/main.svelte @@ -0,0 +1,7 @@ + + +{#each data as d (d)} + {d} +{/each} diff --git a/packages/svelte/tests/runtime-legacy/samples/keyed-each-prod-unique/_config.js b/packages/svelte/tests/runtime-legacy/samples/keyed-each-prod-unique/_config.js new file mode 100644 index 000000000000..2ad5c1b3efc0 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/keyed-each-prod-unique/_config.js @@ -0,0 +1,12 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + test({ assert, target }) { + let button = target.querySelector('button'); + + button?.click(); + + assert.throws(flushSync, /each_key_duplicate/); + } +}); diff --git a/packages/svelte/tests/runtime-legacy/samples/keyed-each-prod-unique/main.svelte b/packages/svelte/tests/runtime-legacy/samples/keyed-each-prod-unique/main.svelte new file mode 100644 index 000000000000..38acbdee7804 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/keyed-each-prod-unique/main.svelte @@ -0,0 +1,8 @@ + + + +{#each data as d (d)} + {d} +{/each}