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

Missed update in diamond-derived stores #613

Closed
igorkamyshev opened this issue Jan 30, 2022 · 2 comments
Closed

Missed update in diamond-derived stores #613

igorkamyshev opened this issue Jan 30, 2022 · 2 comments
Assignees
Labels
bug Something isn't working core effector package fork api

Comments

@igorkamyshev
Copy link
Member

What is the current behavior:

$store.update missed in some narrow case. We can see only one watcher's trigger.

Please provide the steps to reproduce and if possible a minimal demo of the problem via https://share.effector.dev, https://codesandbox.io or similar

https://share.effector.dev/2IjuEI7T

What is the expected behavior:

Two watchers' triggers — initial and first update.

Which versions of effector packages, and which browser and OS are affected by this issue? Did this work in previous versions of effector?:

22.1.1 and later.

Notes

This example will be working correctly if we add scope.getState-call or remove combine call.

@igorkamyshev igorkamyshev added bug Something isn't working needs triage labels Jan 30, 2022
@AlexandrHoroshih
Copy link
Member

I did some research and found out that mapped store $search in the repro is computed twice, while being computed once before 22.1.1

https://share.effector.dev/LhGu9Dlw

Looks like the "flow" of the bug is something like this

  1. Computation starts and $search computed first time

  2. $search is initialized in scope

  3. Then next combined store is computed and during that $search is computed second time for some reason, even though it is already calculated in this run and its dependency was not changed second time 🤔

  4. This part i tested locally with effector repo: - internal update filter $search called only one time after all these computations and compares next value with the current - but since store was computed twice in this run, it compares the same values. This can be proved by changing $search mapper function to return a new reference each update (an object for e.g.) - https://share.effector.dev/qZbb1B18 - this way bug is not appearing

Also calling scope.getState($search) somehow fixes $search computations and it is called only one time - probably because it is initializes value in scope before any computations? 🤔
https://share.effector.dev/N4AeCpYX

@zerobias zerobias added core effector package fork api and removed needs triage labels Feb 1, 2022
@zerobias zerobias self-assigned this Feb 1, 2022
@zerobias zerobias added this to the effector 22.2.0 milestone Feb 1, 2022
zerobias added a commit that referenced this issue Feb 1, 2022
this commit should fail

related issue #613
zerobias added a commit that referenced this issue Feb 1, 2022
it turns out that `combine` read the data too early, which caused data races in the case of diamond deps

related issue #613
@zerobias
Copy link
Member

zerobias commented Feb 1, 2022

Thanks for the report! The fix will be published in next minor release

@zerobias zerobias closed this as completed Feb 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working core effector package fork api
Projects
None yet
Development

No branches or pull requests

3 participants