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

[Question] use with concurrent mode #426

Closed
Jack-Works opened this issue Apr 16, 2021 · 13 comments
Closed

[Question] use with concurrent mode #426

Jack-Works opened this issue Apr 16, 2021 · 13 comments

Comments

@Jack-Works
Copy link

I try to use jotai as a concurrent-mode library but I failed to produce the result I want as I'm following https://reactjs.org/docs/concurrent-mode-patterns.html#splitting-high-and-low-priority-state

Expected:

When I type in the input, the result is updated with staled data of searchResultAtom because I'm updating it inside a startTransition.

Actual

The App goes into the full-suspended state and displays the fallback. isPending never becomes true.

How can I use jotai with concurrent mode correctly?

import React, { useState, unstable_useTransition } from 'react';
import { useAtom, atom } from 'jotai';

const searchingAtom = atom('');
const searchResultAtom = atom(async (get) => {
  const count = get(searchingAtom);
  // simulate API delay
  await new Promise((resolve) => setTimeout(resolve, 1000));
  return count;
});

function App() {
  const [searchingWord, setSearchSuspense] = useAtom(searchingAtom);
  const [searchWord, setSearch] = useState(searchingWord);
  const [result] = useAtom(searchResultAtom);
  const [startTransition, isPending] = unstable_useTransition();
  return (
    <div className="App">
      <header className="App-header">
        <input
          type="text"
          value={searchWord}
          onInput={(e) => {
            setSearch(e.currentTarget.value);
            startTransition(() => {
              setSearchSuspense(e.currentTarget.value);
            });
          }}
        />
        <br />
        <span style={isPending ? { color: 'gray' } : undefined}>{result}</span>
      </header>
    </div>
  );
}

export default function () {
  return (
    <React.Suspense fallback="Loading...">
      <App />
    </React.Suspense>
  );
}
@Thisen
Copy link
Collaborator

Thisen commented Apr 16, 2021

It's important to note that unstable_useTransition is unstable API and we haven't tested Jotai with this API at all.

However, it's interesting to know to keep us ready for concurrent mode, can you add a Codesandbox reproduction @Jack-Works?

Thanks in advance.

@Jack-Works
Copy link
Author

Jack-Works commented Apr 16, 2021

Oh, I thought this library is concurrent-mode-ready...

Code sandbox: https://codesandbox.io/s/quirky-gauss-t8s02 https://codesandbox.io/s/happy-frost-skb09?file=/src/App.js

You may want to know contracts of working in concurrent mode correctly: facebook/react#17526 (comment)

@Thisen
Copy link
Collaborator

Thisen commented Apr 16, 2021

@Jack-Works, as ready as we can be, but we don't have any tests using the experimental branch of React.

@Jack-Works
Copy link
Author

Hi @Thisen Thou I don't know if I'm implementing it right, there is my own example of how should things work https://codesandbox.io/s/dreamy-elion-0kzkl
Click revalidate will trigger an update out of startTransition so it will render Suspense fallback. Click revalidate in concurrent mode will use startTransition and get a better visual effect.

@dai-shi
Copy link
Member

dai-shi commented Apr 16, 2021

Oh, I thought this library is concurrent-mode-ready...

Well, it's hard to define what is CM-ready. Anyway, @Jack-Works this is nice catch. I will investigate it further. Thanks for the codesandbox too.

As @Thisen noted, we don't run our tests in the experimental build, so in one aspect it's not yet supported. We do support <Suspense> in Legacy Mode. (But, note it's technically unstable.)
One the other hand, I run some experiments with https://github.com/dai-shi/will-this-react-global-state-work-in-concurrent-mode, and isPending seems to work fine for sync atoms.
So, the issue is the async atom usage in Concurrent Mode. I'd say it's something between a bug and a feature request.

I would like to find a proper usage, and possibly fix it. However, note that I don't think this is blocking v1 release anyway. For example, if we fix it now, it may not work in the future version of React.

I'll revisit the "contracts" and check the implementation. If anyone noticed anything, it would be good to share here.

@Jack-Works
Copy link
Author

isPending seems to work fine for sync atoms

How sync atoms trigger suspense? Does it throw a Promise?

@dai-shi
Copy link
Member

dai-shi commented Apr 16, 2021

How sync atoms trigger suspense?

When render takes too much time, it will suspend. There's nothing special done in the library. That's how time slicing was working when I tested some time back. As it seems React is changing the (default?) behavior, I'd rather sit and wait until it's settled.

@dai-shi
Copy link
Member

dai-shi commented Apr 16, 2021

@Jack-Works
It seems like you forgot to enable concurrent mode.

ReactDOM.unstable_createRoot(rootElement).render(<App />);

And this is something working based on your example.
https://codesandbox.io/s/happy-frost-skb09?file=/src/App.js

If the original example should work, there might be a timing issue in jotai.

even if we fix it now, it may not work in the future version of React.

But, I'm afraid of this. Better to wait for an official doc.

Please let me know if you find anything while playing with it.

@Jack-Works
Copy link
Author

It seems like you forgot to enable concurrent mode.

Oh I'm sorry 😂

So I tested on the behavior in the concurrent mode, doesn't look correct either.

Reproduce: Type any character, for example, "abcd". Every character should have a 500ms span between them.

And here is the timeline of expected output and actual output. The intermediate results are not displayed

image

@dai-shi
Copy link
Member

dai-shi commented Apr 16, 2021

As jotai's setValue is mutating a ref, this is probably not possible by design.
My former lib use-atom would work like that, because it's truly useState, but I gave up that route. I call the behavior "state branching" and unless react provided some new function to expose it, it would never be possible. I might be wrong.

@Jack-Works
Copy link
Author

Yes, it is possible. There's two undocumented new API for this (redux-like) case:

unstable_createMutableSource and unstable_useMutableSource. The key idea is versioning, for each version (whatever it is), you should give React an immutable snapshot of the current state then React will handle those "state branching" for you.

Here is an example of this API (thou I don't know if it is correct):
https://github.com/DimensionDev/Maskbook/blob/03ba69d882ab0f5e33655444f4fc8b579950762e/packages/shared/src/react/createGlobalState.ts#L39-L39

@dai-shi
Copy link
Member

dai-shi commented Apr 16, 2021

Hm, that was my original expectation and eventually thought uMS alone doesn't allow state branching while experimenting use-context-selector v2.
Maybe what you are referring and state branching is different.

Our plan is to migrate to uMS (real one), so if that solves your use case, it would be good.
...I tried it now, but no, it didn't fix it, unfortunately. Needs more investigation. Does your example with uMS work for state branching? It would be a good hint, then.

(Perhaps, I will update use-atom (making it jotai compatible api) and see how it behaves.)

@dai-shi
Copy link
Member

dai-shi commented May 31, 2021

Let me close this for now and revisit it once React releases a new version.

jotai v1 won't support CM officially, but it should work. And to my knowledge, the current behavior is correct (or best effort) at the moment.

@dai-shi dai-shi closed this as completed May 31, 2021
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

No branches or pull requests

3 participants