crimx / observable-hooks

⚛️☯️💪 React hooks for RxJS Observables. Concurrent mode safe.
https://observable-hooks.js.org
MIT License
1.02k stars 44 forks source link

Specify custom dependencies for `useObservableSuspense` #44

Closed jesster2k10 closed 3 years ago

jesster2k10 commented 3 years ago

I wrote this custom hook that uses the useObervableSuspense to retrieve an Observable<T> value from a custom AnyRepository class (implementation not shown) based on an id string.

export function useFindSuspense(name: RepositoryName, id: string) {
  const repository = useAnyRepository(name);
  const find$ = repository.findById(id);
  const resource = new ObservableResource(find$, value => !!value);

  return useObservableSuspense(resource);
}

The value of the ID is gotten from the react-router (v6) useParams() hook like so:

import {useParams} from 'react-router';

const DetailComponent = () => {
  const deck = useFindSuspense('decks', deckId);
  // render component
};

const Page = () => {
  const {deckId} = useParams()
  // this is wrapped in a Suspense component
  return <DetailComponent deckId={deckId} />
}

and is passed to the custom useFindSuspense hook.

Currently, when the route is changed, the useObservableSuspense hook returns the same value. This is despite the underlying Observable<T> and find$ changing and also the id/deckId input.

I've tried to the following:

  1. Memoize the Observable Resource
export function useFindSuspense(name: RepositoryName, id: string) {
  const repository = useAnyRepository(name);
  const resource = useMemo(() => {
    const find$ = repository.findById(id);
    const observableResource = new ObservableResource(find$, value => !!value);
    return observableResource;
  }, [id, repository]);

  return useObservableSuspense(resource);
}

Note: Adding this line of code:

useEffect(() => {
    console.log('Changed');
  }, [resource]);

prints the Changed to the console as expected (on route change) but the value returned from the hook and the useObservableSuspense hook is unchanged.

  1. Call useEffect to manually update the resource
export function useFindSuspense(name: RepositoryName, id: string) {
  const repository = useAnyRepository(name);
  const find$ = repository.findById(id);
  const resource = new ObservableResource(find$, value => !!value);
  useEffect(() => {
    resource.reload(repository.findById(id));
  }, [id]);

  return useObservableSuspense(resource);
}

Both of which did not do anything, even when the ObservableResource itself changed and it's inputs have changed.

So I assumed that this was because there's no way to specify custom dependencies for the useObservableSuspense hook, which could be "re-calculated" if those dependencies changed.

I will go and try patch the hook to get a working solution, but I think that is what might be nescessary.

jesster2k10 commented 3 years ago

Looking through the source code, I have a feeling that this boils down to https://github.com/crimx/observable-hooks/blob/f0763a343155095c580871850bba2e232fa1b0c8/packages/observable-hooks/src/use-subscription.ts#L67

which leads to https://github.com/crimx/observable-hooks/blob/f0763a343155095c580871850bba2e232fa1b0c8/packages/observable-hooks/src/internal/use-subscription-internal.ts#L91

and in that useEffect or useCustomEffect hook, only the first parameter is being called upon as a dependency, excluding any external arguments like the id:string in this case.

Although, since it registers changes with regards to the input$ observable, I don't see why it's not updating.

jesster2k10 commented 3 years ago

Actually to add to that, https://github.com/crimx/observable-hooks/blob/f0763a343155095c580871850bba2e232fa1b0c8/packages/observable-hooks/src/internal/use-subscription-internal.ts#L34 sets the argsRef value once-off without responding to changes in the args param itself which could explain why this issue is arising.

What actually is happening is that on this line here: https://github.com/crimx/observable-hooks/blob/f0763a343155095c580871850bba2e232fa1b0c8/packages/observable-hooks/src/internal/use-subscription-internal.ts#L40 the args value is not being passed as a dependency to the useIsomorphicLayoutEffect hook which means no further updates are registered (or what I assume)

jesster2k10 commented 3 years ago

After adding the following into the useObservableSuspense hook everything works as expected:

  useEffect(() => {
    setState(resource.read());
  }, [resource]);

I'm not sure if there are any implications elsewhere of this code, but so far that has solved the issue for me. I updated it on my fork there. Awaiting comment

crimx commented 3 years ago

So I assumed that this was because there's no way to specify custom dependencies for the useObservableSuspense hook, which could be "re-calculated" if those dependencies changed.

If you want to listen to props or states changes, there is a useObservable hook in observable-hooks which is for converting dependencies into an observable.

crimx commented 3 years ago

If you can make a codesandbox repro etc I am happy to help debug the issue.

jesster2k10 commented 3 years ago

Here's a sandbox showing the issue at hand. I have another one (which I'll setup) where I was able to fix the issue by adding in that useEffect hook. This is how I have my code setup basically. I could be misusing the library, although I do think this usage is pretty standard.

https://codesandbox.io/s/determined-germain-rlvut?file=/src/App.tsx https://rlvut.csb.app

crimx commented 3 years ago

Thanks for the example. I can see a few issues in the code:

  1. The useFindPostSuspense calls repo.findById and new ObservableResource on every rendering. So everytime a new resource is generated.
  2. useFindPostSuspense creates and consumes a suspense resource in the same component. In React the component which is under suspense will be destroyed and an alternative component will be rendered. It is uncommon that a component suspenses itself to wait for itself.

I think the cause of the issue is that you were trying to use ObservableResource as a hook which it isn't (hooks generally starts with 'use').

An ObservableResource is designed to match only one observable. Technically it is possible to support switching observable source, but I think it's unnecessary since RxJS operators are just more flexible. You can do all kinds of operations over multiple observables like merge or concat.