facebookexperimental / Recoil

Recoil is an experimental state management library for React apps. It provides several capabilities that are difficult to achieve with React alone, while being compatible with the newest features of React.
https://recoiljs.org/
MIT License
19.62k stars 1.19k forks source link

[Feature request] selector support for useRecoilTransaction() #1098

Open dgrechka opened 3 years ago

dgrechka commented 3 years ago

I have the following issue.

Let's say I have atom A with value 1.

1) In mouse event handler the recoil callback with setter is called to set A value to 2. 2) setTimeout callback fires, which in turn contains the recoil callback call. the snapshot in it still contains the A value of 1. The result of first step is not yet applied.

My question. Can I force all of the updates of the recoil callback to be applied right after the callback exits. e.g explicitly force not to batch it into a single transaction with subsequent callback calls ?

So subsequent callback (step two described above) call can read back the written value of previous callback call.

drarmstr commented 3 years ago

It sounds like you are asking about the observability of changes within the callback itself, versus the timing changes are applied to the React component tree. For this, try out the new useRecoilTransaction_UNSTABLE() API #1085 in the nightly build. Other options that may suffice with the current hook are using the updater form of setter by providing a callback which gets the "current" value of the state as a parameter value. Another option may be calling another callback from useRecoilCallback() from the timeout to get an updated Snapshot.

dgrechka commented 3 years ago

Thank you for the feedback! much appreciated!

Indeed, I need the changes made by one callback to be visible by another callback. Both callbacks are obtained via seperateuseRecoilCallback calls.

As for the proposed options.

  1. Seems that useRecoilTransaction_UNSTABLE can help for my use case. Am I right that after callback obtained via useRecoilTransaction_UNSTABLE exits, the changes made by it become visible to the subsequent callbacks calls (obtained either via useRecoilCallback or via useRecoilTransaction_UNSTABLE)? UPDATE: I tried useRecoilTransaction_UNSTABLE from nightly build and I got the error Setting selectors within atomicUpdate is not supported. Is it not ready yet or is it the intended behaviour?
  2. Calling updater instead of providing new value in setter is not an option for me. In my recoil callback I read the data through one selector, and write back the data into recoil through another selector. Although these selectors are based on the same set of underlying atoms, they provide the data in different forms (types). I can't express the update in form of "prevValue => newValue" lambda.
  3. Could you elaborate on this option, please? It seems that I already have it but it does not work as expected.

The toy example of my case is the following.

const callback1 = useRecoilCallback(({snapshot,set}) => ... => {
   ...
   set(atomA, 2);
   console.log('atomA value updated to 2 in the first callback');
   ...
}) // this one is called in mouse event handler,  it is executed first.

const callback2 = useRecoilCallback(({snapshot,set}) => ... => { 
 console.log(`read back atomA value "${snapshot.getLoadable(atomA).GetValue()}" in the second callback`)
}) // this callback is called setTimeout, it fires after the callback1 call within mouse event handler

In the console I see

atomA value updated to 2 in the first callback
read back atomA value 1 in the second callback
dgrechka commented 3 years ago

When I added

useRecoilTransactionObserver_UNSTABLE(({ snapshot, previousSnapshot }) => {
    const getVal = (s: Snapshot) => s.getLoadable(atomA).getValue();
    console.log('recoil transaction. atom was ', getVal(previousSnapshot), '. now it is', getVal(snapshot));
  });

to better understand when the writes are applied, I can see that recoil transaction. atom was 1, now it is 2 appears in the console only after both callbacks are executed. As I understand, what I need is to see this happening in between them.

Note that it is rare case! There is a race condition here. Most of the time I can see the writes of the first callback in the second one. But sometimes they are not applied and I see stale values.

I can imaging the API like useRecoilForceCommitTransaction which returns callback of () => void type. This callback internally apply the accumulated updates, so the next useRecoilCallback obtained callback call is guaranteed to get the snapshot with all of the pending updates applied. Does it make sense?

dgrechka commented 3 years ago

After doing a lot of tracing I found that effect that promotes nextTree to the currentTree inside store may not be called after callback1 exits. React does not do render immediately after callback1 exists, which is absolutly fine. Maybe it decides to postpone the render to do some state update batching

Meanwhile callback1 prepared the nextTree to be applied. timeout with callback2 fires before the effect runs, thus recoil changes are still held in the nextTree and the callback2 reads the old snapshot (based on stale currentTree).

So what I need is

  1. either ability to force endBatch explicitly before callback2 (e.g. with useRecoilFlushBatchedUpdates which returns callback of () => void type).

  2. or an ability to switch useCallback to capture snapshot not based on currentTree, but on the nextTree instead (e.g. add pendingSnapshot to the RecoilCallbackInterface ).

UPDATE: I defined and exported the hook inside Recoil_RecoilRoot.react.js

/** Returns a function that forces accumulated recoil state updates to be applied to `currentTree`.
 *  Enables future `useRecoilCallback` calls (those which will be called before the next render)
 *  to read the updates introduced by recoilCallback calls called since the recent render. */
function useRecoilFlushBatchedUpdates() {
  const storeRef = useStoreRef();
  return useCallback(() => {
    endBatch(storeRef);
  }, [storeRef]);
}

Calling this flushing callback before callback2 resolved my issue! I'm pretty sure, that this solution is not good 🙄. But it highlights the issue.

drarmstr commented 3 years ago

It sounds like the useRecoilTransaction_UNSTABLE() API #1085 would be a good solution for you, then, if it supported selectors. This support is part of the vision for Recoil transactions.

salvoravida commented 3 years ago

@dgrechka "In my recoil callback I read the data through one selector, and write back the data into recoil through another selector."

I think that this is the point: you should not reading from A and setting B, but instead, you should create a new selector C that reactive to that changes.

It's like reading from db while may are pending transactions and updating another db view from that reading. you should correct the view.

salvoravida commented 3 years ago

Meanwhile, @drarmstr great job with useRecoilTransaction!

yoavniran commented 3 years ago

Am I right to think that useRecoilTransaction would/should be the only way to write setters with multiple "set" calls? If so, its really missing the option to get from selectors (and not just atoms). Is there a release date we can expect this to become available? thanks!

kitnato commented 2 years ago

This is definitely a feature I am missing in Recoil right now. A large application will likely have the requirement to get and set various atoms and selectors in one reusable "transaction" (akin to a traditional reducer), and currently I've come across only really one way to achieve this, which is via vanilla React hooks. There are several problems using traditional hooks in this way:

  1. There is a lot of boilerplate involved in initializing all the values and setters of the atoms and selectors
  2. The hook always needs to return a function that does the setting, which requires wrapping it in a useCallback with all its dependencies (lots of boilerplate again)
  3. The only way to get close to the functionality as suggested by useRecoilTransaction is a selector with a setter that does all the work. However, it needs an arbitrary getter which results in confusing and ugly patterns.

It's entirely possible I am going about this wrong of course - anyone else have a solution akin to useRecoilTransaction but with selectors?

I'd also like to echo @yoavniran and others here from over a year ago; is there a roadmap to move Recoil transactions from UNSTABLE to a permanent feature including the use of selectors? Thanks to @drarmstr and the dev team for all your efforts!

EDIT: typos & grammar