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

Overzealous change detection #2768

Closed
Rich-Harris opened this issue May 15, 2019 · 8 comments
Closed

Overzealous change detection #2768

Rich-Harris opened this issue May 15, 2019 · 8 comments
Labels

Comments

@Rich-Harris
Copy link
Member

#2694 introduced a new bug — Svelte is now over-eager when marking values as having changed. The bug can be seen in this demo — it's not present in 3.2.0.

Workaround in the meantime appears to be to manually reorder reactive declarations.

@benjamingr
Copy link

I'm definitely running into this :]

@benjamingr
Copy link

Wouldn't it be better to invalidate the property and not the object when it is statically safe to do so?

That is, I was surprised o.a += 1 invaldiates o and not o.a like in vue or mobx. I realize it's not possible to do for the general case but for property access in assignment I think it's relatively safe.

@Conduitry
Copy link
Member

Looks to be probably the same as #2748 and #2873, although as those issues show, this is also a problem when there aren't multiple reactive declarations. I don't know which one we want to consider the main issue for this, but I had done a little analysis in #2873.

@Panya
Copy link
Contributor

Panya commented Jun 20, 2019

It was fixed in 3.4.4 and above.

@Conduitry
Copy link
Member

This issue was being masked in 3.4.4 and above by #2865, but it looks like it would be actually fixed by #3533.

@Conduitry
Copy link
Member

Fixed in 3.10.1.

@benjamingr
Copy link

benjamingr commented Sep 8, 2019

@Conduitry sorry for the (probabably dumb) question - but how was it fixed in 3.10.1? Can you link to a PR or issue?

(asking for purely intellectual purposes to learn what svelte ended up doing)

@Conduitry
Copy link
Member

I mentioned it in my previous comment - it was fixed via #3533.

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

4 participants