fix(utils): re-implement derive to make glitch free (#335 was buggy)

[Noitidart]

Related: #296 (comment)

• add failing test @Noitidart
• re-implement @dai-shi
• refactor @dai-shi

[vercel]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/pmndrs/valtio/5hbgLdq3Gx5kJ1p3t9m4ZhpjuMVt
✅ Preview: https://valtio-git-fork-noitidart-patch-3-pmndrs.vercel.app

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit a553473:

Sandbox	Source
React	Configuration
React Typescript	Configuration
React Browserify	Configuration
React Snowpack	Configuration
React Parcel	Configuration

[dai-shi]

We should count commits not renders. Let me tweak the test.

[Noitidart]

Oh very interesting I didn't know useEffect was the way to count commits! I learned a lot here thanks!

[dai-shi]

This should be 2, right?

[Noitidart]

Oh yes! Sorry, I made it 3 because thats the count when it was glitched. Thanks for helping fix!

[dai-shi]

Okay, I kind of new my impl in #335 was truly correct. I had another impl, and based on it, a commit is added. It's complicated, so let me try some refactoring.

Meanwhile, @Noitidart could you try if it solves the issue?

[dai-shi]

Let's release this soon, because #342 is a bigger issue. v1.2.10 is full of bugs. 😓

[dai-shi]

Thanks for your investigation. I hope this is better than #335, but not sure if there are some bugs remaining. Please report issues if anything goes wrong.

[Noitidart]

Okay, I kind of new my impl in #335 was truly correct. I had another impl, and based on it, a commit is added. It's complicated, so let me try some refactoring.

Meanwhile, @Noitidart could you try if it solves the issue?

Trying now! Sorry for delay I'm on west coast us so just woke up haha.

[Noitidart]

With v1.2.9 this used to happen:

​compute1 
compute2 
compute3: 
000 
render value: 
v0: 0, v1: 0, v2: 0 
INCREMENT
compute1 
compute3: 
110 
compute2 
compute3: 
111 
render value: 
v0: 1, v1: 1, v2: 1 

Notice how compute1 then compute3 happens, then again compute2 happens then compute3 happens.

Now in v1.2.11 its perfect:

compute1 
compute2 
compute3: 
000 
render value: 
v0: 0, v1: 0, v2: 0 
INCREMENT
compute1 
compute2 
compute3: 
111 
render value: 
v0: 1, v1: 1, v2: 1 

compute1 then compuet2 then compute3 and all variables are lined up! no extra callback is triggered!

[dai-shi]

Trying now! Sorry for delay I'm on west coast us so just woke up haha.

No worries! I knew the timezone difference. I wanted to replace troublesome v1.2.10 as soon as possible.

[Noitidart]

Totally gotcha, thanks for giving me chance to take part of the emergency release. If possible it happens again no matter time if I see it I would definitely get on it, thanks for trying to include us man!

[dai-shi]

By the way, this implementation might still have a limitation. A circular graph may result in an infinite loop. Would you like to test it??
(It may happen with v1.2.9 too.)

[Noitidart]

By the way, this implementation might still have a limitation. A circular graph may result in an infinite loop. Would you like to test it?? (It may happen with v1.2.9 too.)

Haha thanks! I don't understand the circular graph situation, if you could help me understand I would learn about this circular graph thing.

[dai-shi]

The circular graph is something like this: A derived from B derived from C derived from A.

[dai-shi]

Found this: https://en.wikipedia.org/wiki/Reactive_programming#Cyclic_dependencies

It's called cyclic dependencies.

[Noitidart]

Oh thanks! Sorry I got busy with urgently implementing a Stripe payment system update. I'll have to revisit this please. Thanks very much for trying to get me involved!