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

Tries to set computed properties, leading to errors in dev mode #893

Closed
ekhaled opened this issue Oct 12, 2017 · 4 comments
Closed

Tries to set computed properties, leading to errors in dev mode #893

ekhaled opened this issue Oct 12, 2017 · 4 comments
Labels

Comments

@ekhaled
Copy link
Contributor

ekhaled commented Oct 12, 2017

This REPL https://svelte.technology/repl?version=1.40.2&gist=06af5fed951e8b2931e9cfe65aa08e00

It tries to set the value of a computed property available_roles in the generated code.
While it works in the REPL, when you try to run it locally in dev mode, it throws errors Cannot set read-only property 'available_roles' on the console and causes unpredictable behaviour.

This does not happen when the bind:value in the <select> element is not a nested peoperty (i.e does not contain a dot . )

@Conduitry
Copy link
Member

Yeah this looks like a bug to me. In the input event handlers, both the user and the available_roles values are being set - and in dev mode, trying to set available_roles throws an error. I don't see why available_roles should be getting set here - which makes me think that the issue is in the array of dependencies fetched in Binding.ts. I don't believe computed properties should ever be passed to .set - I think that .set takes care of re-computing any computer properties itself.

@Conduitry Conduitry added the bug label Oct 12, 2017
@ekhaled
Copy link
Contributor Author

ekhaled commented Oct 13, 2017

Yeah, seems like 2 bugs:

  1. Trying to set computed property
  2. Thinking available_roles needs to be updated, because it's not bound to anything, and the dependent key roles is also not bound to anything

@Conduitry
Copy link
Member

Just took another look at this and wow it got confusing quick. It looks to be related to the fix for #639 / #642. @Rich-Harris When compiling the component in the above REPL link, would you expect indirectDependencies to be Map { 'user' => Set { 'available_roles' } }? (Which it is, and which is why updates to user are also attempting to update available_roles.) I'm still confused by all this, but that relationship doesn't sound right, given what I understand the fix in #642 to be.

Rich-Harris added a commit that referenced this issue Oct 24, 2017
Rich-Harris added a commit that referenced this issue Nov 12, 2017
@Rich-Harris
Copy link
Member

This is fixed in 1.41.3

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

No branches or pull requests

3 participants