Skip to content

implement Dev mode validation of {#each} block argument - disallow Sets, Maps, nonArrayLikes #4419

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Feb 18, 2020
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/compiler/compile/render_dom/wrappers/EachBlock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,9 @@ export default class EachBlockWrapper extends Wrapper {
const snippet = this.node.expression.manipulate(block);

block.chunks.init.push(b`let ${this.vars.each_block_value} = ${snippet};`);
if (this.renderer.options.dev) {
block.chunks.init.push(b`@validate_each_argument(${this.vars.each_block_value});`);
}

// TODO which is better — Object.create(array) or array.slice()?
renderer.blocks.push(b`
Expand Down
13 changes: 13 additions & 0 deletions src/runtime/internal/dev.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,19 @@ export function set_data_dev(text, data) {
text.data = data;
}

export function validate_each_argument(arg) {
if (arg instanceof Set || arg instanceof Map) {
throw new Error(
`Svelte does not allow Sets or Maps in an {#each} block. Use [...arg.values()] or [...arg.entries()] instead.`
);
}
if (arg.length === undefined) {
throw new Error(
`Svelte needs an array-like value for the {#each} block. You can spread your iterable into an array instead, e.g. [...iterable]`
);
}
}


type Props = Record<string, any>;
export interface SvelteComponentDev {
Expand Down
4 changes: 3 additions & 1 deletion test/js/samples/debug-foo-bar-baz-things/expected.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ import {
safe_not_equal,
set_data_dev,
space,
text
text,
validate_each_argument
} from "svelte/internal";

const file = undefined;
Expand Down Expand Up @@ -88,6 +89,7 @@ function create_fragment(ctx) {
let t1;
let t2;
let each_value = /*things*/ ctx[0];
validate_each_argument(each_value);
let each_blocks = [];

for (let i = 0; i < each_value.length; i += 1) {
Expand Down
4 changes: 3 additions & 1 deletion test/js/samples/debug-foo/expected.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ import {
safe_not_equal,
set_data_dev,
space,
text
text,
validate_each_argument
} from "svelte/internal";

const file = undefined;
Expand Down Expand Up @@ -82,6 +83,7 @@ function create_fragment(ctx) {
let t1;
let t2;
let each_value = /*things*/ ctx[0];
validate_each_argument(each_value);
let each_blocks = [];

for (let i = 0; i < each_value.length; i += 1) {
Expand Down
4 changes: 3 additions & 1 deletion test/js/samples/debug-no-dependencies/expected.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ import {
noop,
safe_not_equal,
space,
text
text,
validate_each_argument
} from "svelte/internal";

const file = undefined;
Expand Down Expand Up @@ -64,6 +65,7 @@ function create_each_block(ctx) {
function create_fragment(ctx) {
let each_1_anchor;
let each_value = things;
validate_each_argument(each_value);
let each_blocks = [];

for (let i = 0; i < each_value.length; i += 1) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
export default {
compileOptions: {
dev: true
},
error: `Svelte does not allow Sets or Maps in an {#each} block. Use [...arg.values()] or [...arg.entries()] instead.`
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<script>
const foo = new Set([1,2,3])
</script>

{#each foo as item}
<div>{item}</div>
{/each}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
export default {
compileOptions: {
dev: true
},
error: `Svelte needs an array-like value for the {#each} block. You can spread your iterable into an array instead, e.g. [...iterable]`
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<script>
const foo = new Set([1,2,3])
</script>

{#each foo.values() as item}
<div>{item}</div>
{/each}