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

Radio/checkbox input with bind:group and spread props makes variable undefined #3680

Closed
richarddewit opened this issue Oct 10, 2019 · 3 comments · Fixed by #4398
Closed

Radio/checkbox input with bind:group and spread props makes variable undefined #3680

richarddewit opened this issue Oct 10, 2019 · 3 comments · Fixed by #4398
Assignees
Labels

Comments

@richarddewit
Copy link

richarddewit commented Oct 10, 2019

Describe the bug
When using an input (type="radio" or type="checkbox") with bind:group in combination with spread properties, it sets the bound variable to undefined when clicked.

To Reproduce
https://svelte.dev/repl/eded09217d054d5cb518712f08833437?version=3.12.1

Severity
Semi-minor inconvenience. Having to specify each prop now instead of using a spread. And not being able to share props easily.

@richarddewit richarddewit changed the title Radio input with bind:group and spread props makes variable undefined Radio/checkbox input with bind:group and spread props makes variable undefined Oct 10, 2019
@Conduitry
Copy link
Member

Conduitry commented Oct 22, 2019

I think this may be a (slightly more convoluted) manifestation of the same issue as #3764. The presence of the spread is preventing the attributes from being set the way they normally would - which in this case includes the special __value property that Svelte then tries to refer to later.

@Conduitry
Copy link
Member

#3764 has been fixed, but, as I noted in a comment there, this is actually a slightly different issue. In situations like this with indirectly bound values, we need to always set the custom __value property on the node. Easiest would probably be to expose the is_indirectly_bound_value value as a method on AttributeWrapper, and to refer to that in ElementWrapper#add_spread_attributes() and emit that code as a special case. I'm not sure where relative to the other updates this should go. It might be that, for correctness, we need to do it inline with the one big set_attributes() call, and that that needs to have a check that always sets __value as a property, even though it's not present on the prototype.

cc @mrkishi

@Conduitry
Copy link
Member

This now works in 3.19.0. Note that the checkboxes still have a different problem (because of the duplicated values), which is documented in #4397, and which I'm not sure how we want to handle.

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

Successfully merging a pull request may close this issue.

2 participants