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

initializeWithDefaultKeyStates overrides data that exists in the Onyx cache #125

Closed
neil-marcellini opened this issue Apr 19, 2022 · 3 comments
Assignees
Labels

Comments

@neil-marcellini
Copy link
Contributor

neil-marcellini commented Apr 19, 2022

Problem

When Onyx.clear() is called the storage is cleared and then Onyx is initialized with the defaultKeyStates provided by the application. In NewDot transition flows, when a user is signing into a different account, Onyx is cleared to sign out the old user and then the new user is immediately signed in, setting a new authToken on the session. By the time initializeWithDefaultKeyStates is called, the new user has been signed in.

In the method, the defaultKeyStates are merged into the key states existing in storage, which are all null after clearing the storage. Then keyChanged is called for each (key, value) pair from the defaultKeyStates. The default key state for session is {loading: false, shouldShowComposeInput: true}, and therefore the subscribers are notified that the session has been updated to this value, and the user is signed out.

Here you can see how the currentValue from the cache contains the full, merged, session information, but the val from defaultKeyStates only contains limited information for initializing.
image

Solution

Notify the subscribers with merged cache data when initializing with default key states.

@marcaaron
Copy link
Contributor

I think this bug needs to be reframed a bit.

I would actually expect a call to Onyx.clear() to be blocking in that if we attempted to set any new data to Onyx while we were clearing it would throw these updates away to prevent cache pollution and cause the conflicts we are describing here.

If you need to set data after storage and cache is cleared then the obvious answer is to just wait for the storage and cache to clear then add the new data.

The problem with the solution and linked PR is that we are making assumptions about the data we are setting when Onyx.clear() is running. Is it something that should be in the store after Onyx.clear() finishes or not? Was it from the previous user session or meant for the next?

@neil-marcellini
Copy link
Contributor Author

After a lot of discussion in Slack we agreed on a solution where we set the default key states while clearing the cache, and remove the call to initializeWithDefaultKeyStates.

@marcaaron
Copy link
Contributor

Fixed in #129

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

2 participants