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

Onyx.update() should return a Promise #205

Closed
marcaaron opened this issue Oct 27, 2022 · 0 comments
Closed

Onyx.update() should return a Promise #205

marcaaron opened this issue Oct 27, 2022 · 0 comments
Assignees

Comments

@marcaaron
Copy link
Contributor

Most other Onyx methods return a Promise so this feels like an implementation oversight. However, there's also a practical advantage in that you might want to apply some updates in an atomic way and it's not possible due to the way mergeCollection() is implemented.

e.g.

Onyx.update([
    {onyxMethod: 'merge', value: {isLoading: false}, key: 'someKey'},
    {onyxMethod: 'mergecollection', value: {test_1: {a: 'a'}}, key: 'test_'},
]);

Although called at the same time, this code will notify the subscriber of someKey first and we might assume the data from mergecollection is available as well - but there are no guarantees about this (and it is most likely not available).

If Onyx.update() returns a promise we can guarantee the correct order of events by doing something like:

Onyx.update(dataUpdate).then(() => Onyx.update(successUpdate));

This is much more predictable and will help solve the bug described here: Expensify/App#11726 (comment)

An future improvement could also be to make these updates atomic somehow - but I'm less sure what the solution for this would look like given the current state of Onyx's API.

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

No branches or pull requests

1 participant