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

Infinite loop while binding an array to a child component #4325

Closed
j3rem1e opened this issue Jan 27, 2020 · 10 comments · Fixed by #4343
Closed

Infinite loop while binding an array to a child component #4325

j3rem1e opened this issue Jan 27, 2020 · 10 comments · Fixed by #4343
Assignees

Comments

@j3rem1e
Copy link

j3rem1e commented Jan 27, 2020

Describe the bug

I have two components :

Cpn.svelte

<script>
  export let items = [];
</script>

and a consumer :

<script>
  import Cpn from './Cpn';
  let items = []
</script>

<Cpn {items} bind:items/>

this consumer has a "bug" : it injects {items} and creates a binding with items. However svelte doesn't output any warning, and the generated code has an infinite loop when the component is rendered.

It "worked" in svelte 3.12

The bug is not really in svelte but in my code, however it's a "hard to debug" case, because chrome/firefox just crash every time the component is shown (and it worked in 3.12).

To Reproduce

I can't provide a REPL because this bug crashes the app.

Expected behavior

The generated code doesn't have an infinite loop/svelte emits an error.

Information about your Svelte project:

  • Chrome 79
  • Windows 7
  • Svelte 3.18.0
  • Webpack
@stephane-vanraes
Copy link
Contributor

Svelte detects when you add the same attribute twice:

<Component value="1" value="2" />

Will correctly give you the error: Attributes need to be unique

However

<Component value="1" bind:value />

Does not give the same error, although I think it should considering it's somehow also a duplicate attribute.

@Conduitry
Copy link
Member

Yeah, I agree that foo={...}, {foo}, and bind:foo should probably all be mutually exclusive, and using any two of them should be a compile-time error.

Conduitry added a commit to Conduitry/sveltejs_svelte that referenced this issue Jan 30, 2020
@Conduitry Conduitry self-assigned this Jan 30, 2020
Conduitry added a commit that referenced this issue Jan 30, 2020
* disallow matching attributes/shorthands/bindings (#4325)

* add tests

* update changelog
@Conduitry
Copy link
Member

In 3.18.2, it's now a compiler error to have any two of foo=..., {foo}, or bind:foo.

@juniorsd
Copy link

juniorsd commented Feb 9, 2020

I think that this should be allowed though.

<input type="text" value={value} bind:value={query} />

@Conduitry
Copy link
Member

I don't know what value={value} bind:value={query} would be used for, or how a component with those on an element would behave.

@juniorsd
Copy link

juniorsd commented Feb 9, 2020

I used this construct in an autocomplete dropdown component, where I wanted to keep the query and the selected value separated.

@pielambr
Copy link

We use this to set an initial value on an input, but at the same time keep it up to date. How would we change that to work with these changes?

@stephane-vanraes
Copy link
Contributor

You would set the bound-to value to an initial value.

let items = ["item1"];

<Cpn bind:items />

@pielambr
Copy link

So bind goes both ways? The documentation would make it seem that it just does one-way, from parent to child. Thank you for pointing that out!

Data ordinarily flows down, from parent to child. The bind: directive allows data to flow the other way, from child to parent.

@stephane-vanraes
Copy link
Contributor

The docs are maybe a bit ambiguous, it could indeed be interpreted as bind reversing the flow, while in reality it allows data to flow both ways.

jesseskinner pushed a commit to jesseskinner/svelte that referenced this issue Feb 27, 2020
* disallow matching attributes/shorthands/bindings (sveltejs#4325)

* add tests

* update changelog
taylorzane pushed a commit to taylorzane/svelte that referenced this issue Dec 17, 2020
* disallow matching attributes/shorthands/bindings (sveltejs#4325)

* add tests

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

Successfully merging a pull request may close this issue.

5 participants