re-rxjs/react-rxjs

react-rxjs hook not executing correctly

sbitproz opened this issue · 7 comments

I'm trying to get an idea from your GitHub example of how I might use your library to manage some data streams to represent an API request including data, isLoading and error. I know I could have 3 separate Subjects and maybe this is the better way to achieve this. However I do think having collating related data makes sense for organising code or accessing related data streams through a single react-rxjs hook on a component. I have many API requests in this app, and would like to re-use this pattern throughout the app.

The state

import { merge, Subject } from "rxjs"
import { shareLatest, bind } from "@react-rxjs/core"
import API from "../api/api"
import ROUTES from "../api/routes"

interface EntityField {
  id: string;
  entityId: string;
  name: string;
  type: "Text" | "Number" | "Checkbox";
  size: "Large" | "Medium" | "Small";
  searchable: boolean;
  required: boolean;
  position: number;
}

const isLoading = new Subject<boolean>();
const toggleLoading = (loading: boolean) => isLoading.next(loading);

const error = new Subject<string>();
const setError = (errorMessage: string) => error.next(errorMessage);

const fields = new Subject<EntityField[]>();
const loadEntityFields = async (entityId: string) => {
  toggleLoading(true);
  try {
    const { data } = await API.get(`${ROUTES.ENTITIES_FIELDS}/${entityId}/fields`);
    fields.next(data);
  } catch (e) {
    setError(e);
    console.error(e);
  } finally {
    toggleLoading(false);
  }
}

const entityFieldsMerge = merge(
  isLoading,
  error,
  fields
).pipe(shareLatest())

const [useEntityFields, entityFields$] = bind(entityFieldsMerge);

const entityFieldsStore = {
  useEntityFields,
  entityFields$,
  loadEntityFields
}

export default entityFieldsStore;

This doesn't seem to work - In the component I have:

const EntityFields: React.FC<EntityFieldsProps> = ({ entityId }) => {
  console.log('entityId ', entityId );
  const classes = useEntityFieldsStyles();

  const fields = entityFieldStore.useEntityFields();
  console.log("fields", fields);    
....

For the subscription I've done:

const provider$ = merge(entityFieldStore.entityFields$, layoutStore.profileSidePanelOpen$, layoutStore.sideMenuOpen$);

function App() {
  return (
    <Subscribe source$={provider$}>
      <ThemeProvider theme={theme}>
        <CssBaseline />
        <Router>
          <Routes />
        </Router>
      </ThemeProvider>
    </Subscribe>
  );
}

The layoutStore related hooks work fine, of course they've very simple boolean observables without any merges, but depending on your response I might refactor this code too.

The console.log("fields", fields); doesn't execute and with the hook useEntityFields the app appears to behave in a peculiar way - it appears to interfere with the current MobX store (but that's probably just a false flag).

Am I approaching this solution in the wrong way?

I'm hoping to change from MobX to the react-rxjs in the event that I can successfully demonstrate a PoC with react-rxjs.

Where do you invoke loadEntityFields to actually trigger the request?

If you use RxJs from on your promise-based api request to turn it into an Observable, you shouldn't have to set up any Subjects. Adapting the example in the fromFetch docs:

https://rxjs-dev.firebaseapp.com/api/fetch/fromFetch

Add a startWith in the pipe that initializes { fields: null, error: null, isLoading: true }, and then from the switchMap and catchError return the same interface with isLoading set to false and the other relevant field set.

Hi @iprado78 thanks for the reply (so swift).

loadEntityField was invoked on the relevant component useEffect.

That is indeed a great idea (to convert the promise based api), so as I'm using Axios which I have now wrapped with from I have the following.

I have used a map, as I believe that there's no inner observable requirement here. I tested this in a tiny project which appears to work correctly.

const loadEntityFields = (entityId: string): any => {
  return from(API.get(`${ROUTES.ENTITIES_FIELDS}/${entityId}/fields`)).pipe(
    startWith({ isLoading: true, error: undefined, data: undefined }),
    map(({ data }) => {
      return { isLoading: false, error: undefined, data }
  }),
    catchError((e) => {
      console.error(e);
      return of({ isLoading: false, error: e, data: undefined })
    })
  )
}

const [useEntityFields, entityFields$] = bind((entityId: string) => loadEntityFields(entityId));  // <<< obviously not possible

const entityFieldsStore = {
  useEntityFields,
  entityFields$
}

export default entityFieldsStore;

Now I know this is not possible, as I have provided a function where a stream was expected.

My question is how would I work this pattern? The Observable is dependent upon the function execution with an argument entityId. Then this needs to be passed into the bind function and <Subscribe source$={provider$}> at execution time?

It seems to me that a Subject might be required at this point which could be updated with the next value on a subscription added to the from(API_REQUEST) function (like the following). What are your thoughts?

const entityFields = new Subject();

const loadEntityFields = (entityId: string): any => {
  return from(API.get(`${ROUTES.ENTITIES_FIELDS}/${entityId}/fields`)).pipe(
    startWith({ isLoading: true, error: undefined, data: undefined }),
    map(({ data }) => {
      return { isLoading: false, error: undefined, data }
  }),
    catchError((e) => {
      console.error(e);
      return of({ isLoading: false, error: e, data: undefined })
    })
  ).subscribe((response) => {
    entityFields.next(response);
 })
}


const [useEntityFields, entityFields$] = bind(entityFields);

const entityFieldsStore = {
  useEntityFields,
  entityFields$,
  loadEntityFields
}

So this is where I've left it, any feedback would be greatly appreciated

interface EntityField {
  id: string;
  entityId: string;
  name: string;
  type: "Text" | "Number" | "Checkbox";
  size: string;
  searchable: boolean;
  required: boolean;
  position: number;
}

export const subscriptionCallback = <T>(subject: Subject<ApiObservable<T>>) => ({ data }) => {
  subject.next({ isLoading: false, error: undefined, data })
};

export const catchErrorCallback = (errorMessage?) => catchError<AxiosResponse<any>,ObservableInput<ApiObservable<any>>>((e) => {
  console.error(errorMessage, e);
  const prefix = errorMessage ? `${errorMessage}:` : '';
  return of({ isLoading: false, error: `${prefix}${e}`, data: undefined })
});

const entityFields = new Subject<ApiObservable<EntityField[]>>();

const entityFieldsSubscription = subscriptionCallback(entityFields);

const loadEntityFields = (entityId: string): any => {
  return from(API.get(`${ROUTES.ENTITIES_FIELDS(entityId)}`)).pipe(
    catchErrorCallback('Load Entity Fields')
  ).subscribe(entityFieldsSubscription);
}

const [useEntityFields, entityFields$] = bind(entityFields.pipe(startWith(apiStartWith)));

const entityFieldsStore = {
  useEntityFields,
  entityFields$,
  loadEntityFields
}

export default entityFieldsStore;

Hi @sbitproz !

First off thanks for opening this issue and for sharing your code. Second, my apologies because I'm afraid that if we had had better docs I wouldn't need to write such a long answer. Because, I'm afraid that I will have to clarify a number of things so that I can properly answer your questions. So, please be open-minded and a bit patient 🙂 . Let's get started:

First things first: one of the goals for this library is to properly leverage React's Suspense and Error boundaries, so that we don't have to constantly track the loading state of our streams. This is something that the "Github Issues Viewer" tutorial of the docs covers in a very shallow way and we should really explain it better. So, generally speaking, there is no need to do what you are trying to do, it's easier to let the React and React-RxJS help you on that one. However, what if you don't want to leverage React's Suspense and/or React's Error Boundaries? Then I suggest that you create a pipeable operator like this one (I have not tried it, but I'm pretty sure that it should work 🙈 ):

import { SUSPENSE } from '@react-rxjs/core'

export enum Status { Loading, Error, Data }
export type WithStatus<T> =
  | { status: Status.Loading }
  | { status: Status.Error, value: any }
  | { status: Status.Data, value: T }

export const withStatus = () => <T>(source: Observable<T>) => new Observable<WithStatus<T>>(observer => {
  let hasEmitted = false
  const subscription = source.subscribe(
    value => {
      hasEmitted = true
      observer.next(
        value === SUSPENSE
          ? { status: Status.Loading }
          : { status: Status.Data, value }
      )
    },
    err => {
      hasEmitted = true
      observer.next({ status: Status.Error, value: err })
    }
  )
  
  if (!hasEmitted) {
    observer.next({ status: Status.Loading })
  }
  
  return subscription
})

And then you could do something like this:

import { ajax } from 'rxjs/ajax'
import { withStatus, WithStatus } from '../utils/withStatus'

export interface EntityField {
  id: string;
  entityId: string;
  name: string;
  type: "Text" | "Number" | "Checkbox";
  size: string;
  searchable: boolean;
  required: boolean;
  position: number;
}

export const getEntityFields$ = (entityId: string): Observable<WithStatus<EntityField[]>> =>
  ajax.getJSON<EntityField[]>(`${ROUTES.ENTITIES_FIELDS}/${entityId}/fields`)
    .pipe(withStatus())

or if one day you decide that you actually prefer to leverage React's Suspense and Error boundaries, you could just do this instead, up to you 😉

import { ajax } from 'rxjs/ajax'

export interface EntityField {
  id: string;
  entityId: string;
  name: string;
  type: "Text" | "Number" | "Checkbox";
  size: string;
  searchable: boolean;
  required: boolean;
  position: number;
}
export const getEntityFields$ = (entityId: string): Observable<EntityField[]> =>
  ajax.getJSON<EntityField[]>(`${ROUTES.ENTITIES_FIELDS}/${entityId}/fields`)

Ok, so, so far so good. Now the real question is: what do we do when we have "parametric" streams?

Because unfortunately, in the tutorial we have not explained that... yet. And that's something super-important. My apologies for that. We have to improve the docs urgently, it's just that it's difficult to find the time. In my defence, it's documented in the API of bind, but that's obviously not good enough. Anyways, hopefully this code will set you on the right track:

import { ajax } from 'rxjs/ajax'
import { memo, createContext } from 'react'
import { bind, Subscribe } from '@react-rxjs/core'
import { ErrorBoundary, FallbackProps } from "react-error-boundary"

const EntityIdContext = createContext<string>();

const OnError: React.FC<FallbackProps> = ({ error, resetErrorBoundary }) => {
  const entityId = useContext(EntityIdContext)
  return (
    <div>
      <h1>Something went wrong while loading EntityId: {entityId}!</h1>
      <div>{error && error.message}</div>
      <button onClick={resetErrorBoundary}>Try loading it again</button>
    </div>
  )
}

interface EntityField {
  id: string;
  entityId: string;
  name: string;
  type: "Text" | "Number" | "Checkbox";
  size: string;
  searchable: boolean;
  required: boolean;
  position: number;
}

const [useEntityFields, getEntityFields$] = bind(
  (entityId: string): Observable<EntityField[]> =>
    ajax.getJSON<EntityField[]>(`${ROUTES.ENTITIES_FIELDS}/${entityId}/fields`)
)

const EntityFieldsList = React.FC = () => {
  const entityId = useContext(EntityIdContext)
  const entityFields = useEntityFields(entityId)
  return (
    <ul>{entityFields.map(field => (
      <li key={field.id}>{field.name}</li>
    ))}</ul>
  )
}

export const EntityFields: React.FC<{ entityId: string }> = memo(({ entityId }) => (
  <EntityIdContext.Provider value={entityId}>
    <ErrorBoundary FallbackComponent={OnError}>
      <Subscribe source$={getEntityFields$(entityId)} fallback={<>Loading {entityId}...</>}>
        <EntityFieldsList />
      </Subscribe>
    </ErrorBoundary>
  </EntityIdContext.Provider>
))

There are a number of things that I would like to explain about the snipped above, like the fact that React-RxJS Subscribe component is in fact a super-set of React's Suspense component, but it's getting pretty late and I'm tired. So, I will review this comment as soon as I can and I will add more context. But hopefully this will set you on the right track. Thanks for opening these issues and for all your feedback.

@josepot thanks for your heroic effort.

I will branch and test the suggestions above and come back to you.

Hey @sbitproz did these suggestions help you out?

I'll close this issue as I think no further action is required, although we can reopen it if that's not the case.

@josepot @voliva sure did. Thanks - really appreciate it.