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

Experiment: Redux with useMutableSource #12

Merged
merged 4 commits into from
Mar 13, 2020
Merged

Conversation

dai-shi
Copy link
Owner

@dai-shi dai-shi commented Feb 21, 2020

useMutableSource is proposed in the RFC.

Let us experiment with it.

@dai-shi
Copy link
Owner Author

dai-shi commented Feb 21, 2020

@dai-shi
Copy link
Owner Author

dai-shi commented Feb 21, 2020

  redux-use-mutable-source
    check with events from outside
      ✓ check 1: updated properly (8379ms)
      ✓ check 2: no tearing during update (2ms)
      ✓ check 3: ability to interrupt render (1ms)
      ✓ check 4: proper update after interrupt (2412ms)
    check with useTransition
      ✓ check 5: updated properly with transition (4560ms)
      ✓ check 6: no tearing with transition (2ms)
      ✕ check 7: proper branching with transition (7425ms)
    check with intensive auto increment
      ✓ check 8: updated properly with auto increment (3006ms)
      ✕ check 9: no tearing with auto increment (3ms)

Unfortunately, check 9 is failing. The failure in check 7 is OK, which is impossible.

@dai-shi
Copy link
Owner Author

dai-shi commented Feb 21, 2020

This is done with this commit with a patch:

diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js
index 375c5e971..5e3f2b587 100644
--- a/packages/react-reconciler/src/ReactFiberHooks.js
+++ b/packages/react-reconciler/src/ReactFiberHooks.js
@@ -1019,7 +1019,7 @@ function useMutableSourceImpl<Source, Snapshot>(
             return prevState;
           }
 
-          setState({
+          return ({
             ...prevState,
             snapshot: newSnapshot,
           });

@salvoravida
Copy link
Contributor

great work!

have you patched useMutableSource or is the last version?

@dai-shi
Copy link
Owner Author

dai-shi commented Feb 21, 2020

Now, I try the latest one: facebook/react@b028a0b

  redux-use-mutable-source
    check with events from outside
      ✓ check 1: updated properly (8717ms)
      ✓ check 2: no tearing during update (5ms)
      ✓ check 3: ability to interrupt render (1ms)
      ✓ check 4: proper update after interrupt (2399ms)
    check with useTransition
      ✓ check 5: updated properly with transition (3675ms)
      ✓ check 6: no tearing with transition (2ms)
      ✕ check 7: proper branching with transition (7444ms)
    check with intensive auto increment
      ✓ check 8: updated properly with auto increment (3192ms)
      ✕ check 9: no tearing with auto increment (3ms)

No difference in result.

@dai-shi
Copy link
Owner Author

dai-shi commented Feb 22, 2020

Hoping to support state branching.

https://github.com/dai-shi/will-this-react-global-state-work-in-concurrent-mode/blob/53f42617c33e039ee2b32aadf339d0537bc17f18/src/redux-use-mutable-source/index.js#L25-L74

  redux-use-mutable-source
    check with events from outside
      ✓ check 1: updated properly (3602ms)
      ✓ check 2: no tearing during update (10ms)
      ✓ check 3: ability to interrupt render (1ms)
      ✓ check 4: proper update after interrupt (2373ms)
    check with useTransition
      ✓ check 5: updated properly with transition (3600ms)
      ✓ check 6: no tearing with transition (2ms)
      ✕ check 7: proper branching with transition (5415ms)
    check with intensive auto increment
      ✓ check 8: updated properly with auto increment (3163ms)
      ✓ check 9: no tearing with auto increment (1ms)

Hmm, it tears with useTransition. (It's failing with check 7, but it's because check 6 is not enough.)

@dai-shi
Copy link
Owner Author

dai-shi commented Feb 23, 2020

I was probably sleeping. State branching won't work because we don't have update queue.

@dai-shi
Copy link
Owner Author

dai-shi commented Mar 13, 2020

As useMutableSource is now in the experimental channel, let's merge this and continue experiments in master.

@dai-shi dai-shi merged commit 35e0c55 into master Mar 13, 2020
@dai-shi dai-shi deleted the redux-use-mutable-source branch March 13, 2020 02:46
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.

2 participants