Skip to content

Conversation

@subtle-byte
Copy link

Fixes #6328

This PR just suggests an idea. Seems like it works, may be useful.

It was tested after applying #6331

Before submitting the PR, please make sure you do the following

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with npm test and lint the project with npm run lint

@benmccann benmccann changed the title Fixing reactivity glitches [fix] Fixing reactivity glitches Jun 30, 2021
@benmccann
Copy link
Member

@subtle-byte it looks like this PR will need to be rebased

@subtle-byte
Copy link
Author

@benmccann It is rebased now

@rmunn
Copy link
Contributor

rmunn commented Sep 17, 2021

I think this needs a lot of testing and user feedback to verify whether it's what people would expect. If I write derived([storeA, storeB]), normally my derived store's callback is called whenever any of the values in any of its input stores have changed. So if storeA changes from 'hello' to 'bye' and then later on storeB changes from 'world' to 'folks', I'd expect to see 'bye world' before I see 'bye folks'. And that's just what's happening here: the lastname store is changing from 'Jekyll' to 'Hyde' first, and then the firstname store changes from 'Henry' to 'Edward', and so we see a 'Henry Hyde' entry first before we see 'Edward Hyde'.

Thing is, I suspect there are probably use cases where that's absolutely the desired behavior, where you want to see both changes. And there are other use cases where you only want to see the change after the dependent store has finished updating. The question is how to accomodate both use cases, and that's why I think it's important to do a lot of testing and get user feedback before merging this PR, to make sure that this is what people would expect.

}
}
} else {
revalidate_writable();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect that this logic might be incorrect; I'll try to come up with proof (i.e., a use case where this ends up donig the wrong thing). Once I come up with an example, I'll put it in a REPL and link it here.

@dummdidumm dummdidumm added this to the one day milestone Mar 2, 2023
@benmccann benmccann changed the title [fix] Fixing reactivity glitches fix: address reactivity glitches Mar 14, 2023
@Rich-Harris
Copy link
Member

Closing Svelte 4 PRs as stale — thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Prevents glitches?

5 participants