Skip to content
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

Dev mode validation of {#each} block argument #4408

Closed
Conduitry opened this issue Feb 13, 2020 · 6 comments · Fixed by #4419
Closed

Dev mode validation of {#each} block argument #4408

Conduitry opened this issue Feb 13, 2020 · 6 comments · Fixed by #4419

Comments

@Conduitry
Copy link
Member

Is your feature request related to a problem? Please describe.
Many people have been surprised when {#each} doesn't work with iterables, and there have been a number of issues about adding support for this - which we don't want to do, for reasons outlined elsewhere.

Describe the solution you'd like
A dev mode only check that makes sure the argument to the {#each} block is an object with a length property. If not, throw a descriptive runtime error about needing an array-like.

If the object in question has a Symbol.iterator property, we could also add a note to the error about [...foo]/Array.from(foo). We'd want to be careful to not accidentally cause a different error in js engines that don't have Symbol.iterator.

This would probably be implemented by writing a new validate_each_argument (name tbd) function in the same vein as validate_store or validate_each_keys and only emit code to call it in dev mode.

Describe alternatives you've considered
Do nothing. It's sort of worked so far.

How important is this feature to you?
The main reason it'd be important to me personally is that it might reduce the number of questions or issues about {#each} and iterables, which would be nice.

Additional context
Not really.

@mikebeaton
Copy link

mikebeaton commented Feb 13, 2020

there have been a number of issues about adding support for this

Incl. #4405 #4289 #3225 #894 (see also #2968)

for reasons outlined elsewhere

Esp. #894 (comment) #894 (comment)

@swyxio
Copy link
Contributor

swyxio commented Feb 13, 2020

haha and i asked for this in #4405

i would be happy to work on this, my first contribution to svelte

@tanhauhau
Copy link
Member

tanhauhau commented Feb 13, 2020

@sw-yx sure 👍

Some pointers to help you started (if you need it 😉) :

  1. you can implement the validate_each_argument helper function here in src/runtime/internal/dev.ts
  2. you need to add it for both dom and ssr
  3. tests goes here for dom and ssr can reuse the same runtime test sample (you can refer to this test sample)

@swyxio
Copy link
Contributor

swyxio commented Feb 13, 2020

yes i needed that help :) thanks vm

@swyxio
Copy link
Contributor

swyxio commented Feb 15, 2020

update: i think since this is dev mode warning only, adding it in SSR is not needed? i looked at other devmode warnings and none of them are applied for SSR

@Conduitry
Copy link
Member Author

There are now hopefully more helpful dev mode errors if your {#each} argument doesn't look like an array-like object, with an additional message if it looks like an iterable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants