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

Redux store vs Redux API queries #4283

Open
MaximusFT opened this issue Mar 20, 2024 · 1 comment
Open

Redux store vs Redux API queries #4283

MaximusFT opened this issue Mar 20, 2024 · 1 comment

Comments

@MaximusFT
Copy link

In our company, a dispute arose on the topic of how and where it is better to store and reuse data. Opinions were divided. Here I will give an example.

We have a queue of requests
The first request is to obtain vehicle data by its Transmission ID.
The second request is to get a gallery of links to pictures for the Interior and Exterior.
The third request is to get links to a gallery of 360 degree images for the Interior and Exterior.

One team's solution was this:

Api config

// bapApi.ts

const bapApi = createApi({
  reducerPath: 'bapApi',
  baseQuery: fetchBaseQuery(),
  endpoints: builder => ({
    getColorByTransmission: builder.query<BapImages | undefined, BapColorByTransmissionQueryArgs>({
      query: ({ url }) => ({ url }),
    }),
    getStudioAssetsById: builder.query<BapImages | undefined, BapStudioAssetsByIdQueryArgs>({
      query: ({ url }) => ({ url }),
    }),
    get360AssetsById: builder.query<BapImages | undefined, Bap360AssetsByIdQueryArgs>({
      query: ({ url, body }) => ({
        url,
        method: 'POST',
        body,
      }),
    }),
  }),
});

Main hook to chain requests

// useFetchLayoutServiceTransmission.ts

const useFetchLayoutServiceTransmission = ({ transmissionId }: { transmissionId?: string }) => {
  const dispatch = useDispatch();
  ...
  useEffect(() => {
    if (transmissionId) {
      dispatch(setTransmissionId(transmissionId));
    }
    return () => {
      dispatch(setTransmissionId(''));
    };
  }, [transmissionId]);

  // Get Transmission Section
  const {
    data: transmissionData,
    isFetching: isFetchingTransmission,
    isLoading: isLoadingTransmission,
    error: errorTransmission,
  } = useGetColorByTransmissionQuery(
    { url: getUrl(`/qwe/${transmissionId}`), transmissionId },
    { skip: !transmissionId },
  );

  // Studio Section
  const { interiorColorId, exteriorColorId } = useSelector(selectTransmissionColors);

  const {
    isFetching: isFetchingExteriorStudioAssets,
    isLoading: isLoadingExteriorStudioAssets,
    error: errorExteriorStudioAssets,
  } = useGetStudioAssetsByIdQuery(
    {
      url: getUrl(`/asd/${exteriorColorId}/studioAssets`),
      id: exteriorColorId,
      type: 'exteriorStudioAssets',
    },
    { skip: !exteriorColorId },
  );

  const {
    isFetching: isFetchingInteriorStudioAssets,
    isLoading: isLoadingInteriorStudioAssets,
    error: errorInteriorStudioAssets,
  } = useGetStudioAssetsByIdQuery(
    {
      url: getUrl(`/zxc/${interiorColorId}/studioAssets`),
      id: interiorColorId,
      type: 'interiorStudioAssets',
    },
    { skip: !interiorColorId },
  );

  // 360 Section
  const { colorIds, hasColors } = useSelector(selectTransmission360Colors);

  const {
    isFetching: isFetching360StudioAssets,
    isLoading: isLoading360StudioAssets,
    error: error360StudioAssets,
  } = useGet360AssetsByIdQuery(
    { url: getUrl('/qaz'), body: colorIds },
    { skip: !hasColors },
  );

  return {
    isFetching:
      isFetchingTransmission ||
      isFetchingExteriorStudioAssets ||
      isFetchingInteriorStudioAssets ||
      isFetching360StudioAssets,
    isLoading:
      isLoadingTransmission ||
      isLoadingExteriorStudioAssets ||
      isLoadingInteriorStudioAssets ||
      isLoading360StudioAssets,
    error: errorTransmission || errorExteriorStudioAssets || errorInteriorStudioAssets || error360StudioAssets,
    transmissionData,
  };
};

Reducer and Selectors

// bapColorImages.ts

const initialState: BapColorImages = {
  transmissions: {},
  transmissionId: '',
  interiorColorId: '',
  exteriorColorId: '',
};

const bapColorImages = createReducer(initialState, builder => {
  builder
    // ...
    .addMatcher(bapApi.endpoints.getColorByTransmission.matchFulfilled, (state, action: PayloadAction<any>) => {
      const selectedExteriorColor = action?.payload?.transmission?.exteriorColors;
      const { transmissionId } = action.meta.arg.originalArgs;
      const ret = {...};

      state.transmissions[transmissionId] = ret;
    })
    .addMatcher(bapApi.endpoints.getStudioAssetsById.matchFulfilled, (state, action: PayloadAction<any>) => {
      const { transmissionId } = state;
      const { id } = action.meta.arg.originalArgs;

      const studioAssetsRet = {
        ...state.transmissions[transmissionId].studioAssets,
        [id]: action.payload.studioAssets,
      };

      const ret = {
        ...state.transmissions[transmissionId],
        studioAssets: studioAssetsRet,
      };
      state.transmissions[transmissionId] = ret;
    })
    .addMatcher(bapApi.endpoints.get360AssetsById.matchFulfilled, (state, action: PayloadAction<any>) => {
      const { transmissionId } = state;

      const three60AssetsRet = {...};

      const ret = {
        ...state.transmissions[transmissionId],
        three60Assets: three60AssetsRet,
      };
      state.transmissions[transmissionId] = ret;
    });
});

// And some selectors

Well, naturally, the component called the Hook and received data from the reducer through selectors.

The second command did this:

Main hook to chain requests

// useFetchLayoutServiceAssets.ts

const useFetchLayoutServiceAssets = (transmissionId?: string) => {
  // ...
  const {
    data: exteriorImages,
    isFetching: isFetchingExteriorStudioAssets,
    isLoading: isLoadingExteriorStudioAssets,
    error: errorExteriorStudioAssets,
    isSuccess: isSuccessExteriorStudioAssets,
  } = useGetStudioAssetsByIdQuery(
    {
      url: getUrl(`/qwe/${exteriorColorId}/studioAssets`),
      id: exteriorColorId,
    },
    { skip: !transmissionData?.exteriorColorId },
  );

  const {
    data: interiorImages,
    isFetching: isFetchingInteriorStudioAssets,
    isLoading: isLoadingInteriorStudioAssets,
    error: errorInteriorStudioAssets,
    isSuccess: isSuccessInteriorStudioAssets,
  } = useGetStudioAssetsByIdQuery(
    {
      url: getUrl(`/asd/${interiorColorId}/studioAssets`),
      id: interiorColorId,
    },
    { skip: !transmissionData?.interiorColorId },
  );

  const threeSixtyDataRequestBody = {
    exteriorItemIds: [transmissionData?.exteriorThreeSixtyId || ''],
    interiorItemIds: [transmissionData?.interiorThreeSixtyId || ''],
  };

  /**
   * Skip ThreeSixty if there are no ThreeSixty Sitecore IDs, or if the
   * studio assets have Sitecore IDs but the requests aren't done yet.
   */
  const shouldSkipThreeSixty =
    !transmissionData?.exteriorThreeSixtyId ||
    !transmissionData?.interiorThreeSixtyId ||
    (!!transmissionData?.exteriorColorId && !isSuccessExteriorStudioAssets) ||
    (!!transmissionData?.interiorColorId && !isSuccessInteriorStudioAssets);

  const {
    data: threeSixtyImages,
    isFetching: isFetching360StudioAssets,
    isLoading: isLoading360StudioAssets,
    error: error360StudioAssets,
  } = useGet360AssetsByIdQuery(
    { url: getUrl('/zxc'), body: threeSixtyDataRequestBody },
    {
      skip: shouldSkipThreeSixty,
    },
  );

  return {
    data: {
      exteriorImages,
      interiorImages,
      threeSixtyImages,
      nameBadge,
    },
    isFetching: isFetchingExteriorStudioAssets || isFetchingInteriorStudioAssets || isFetching360StudioAssets,
    isLoading: isLoadingExteriorStudioAssets || isLoadingInteriorStudioAssets || isLoading360StudioAssets,
    error: errorExteriorStudioAssets || errorInteriorStudioAssets || error360StudioAssets,
  };
};

Api layout

// layoutServiceApi.ts

const layoutServiceApi = createApi({
  reducerPath: 'layoutServiceApi',
  baseQuery: fetchBaseQuery(),
  endpoints: builder => ({
    // ...
    getColorByTransmission: builder.query<TransmissionAssets | undefined, GetColorByTransmissionQueryArgs>({
      query: ({ url }) => ({ url }),
      transformResponse: transformLayoutServiceTransmissionApiResult,
    }),
    getStudioAssetsById: builder.query<LayoutServiceAsset[] | undefined, GetStudioAssetsByIdQueryArgs>({
      query: ({ url }) => ({ url }),
      transformResponse: transformLayoutServiceApiAssetsResult,
    }),
    get360AssetsById: builder.query<LayoutServiceThreeSixtyAssets | undefined, Get360AssetsByIdQueryArgs>({
      query: ({ url, body }) => ({
        url,
        method: 'POST',
        body,
      }),
      transformResponse: transformLayoutServiceThreeSixtyResult,
    }),
  }),
});

And as you can see, the API contains transformResponse for each request, I will not present them here, I think the logic is clear.

Summary

It turns out that one team created a data warehouse, formats everything there as it should, saves it, and takes data from there.

The second team abandoned the Reducer and each time retrieves the transformed data from API queries.

If you compare, there are pros and cons in both options. I would like to know the community’s opinion on what, in your opinion, is the correct way to store data and why?

Additionally, if there are other options, I will be glad to communicate in the comments.

Thank you

@MaximusFT MaximusFT changed the title Redux store vs Redux Query Redux store vs Redux API queries Mar 20, 2024
@phryneas
Copy link
Member

phryneas commented Mar 20, 2024

I don't know where you got that query: ({ url }) => ({ url }), pattern from, but you're absolutely missing the point what the query function is for.

Instead of

  endpoints: builder => ({
    getColorByTransmission: builder.query<BapImages | undefined, BapColorByTransmissionQueryArgs>({
      query: ({ url }) => ({ url }),
    }),

and

  } = useGetColorByTransmissionQuery(
    { url: getUrl(`/qwe/${transmissionId}`), transmissionId },
    { skip: !transmissionId },
  );

do

  endpoints: builder => ({
    getColorByTransmission: builder.query<BapImages | undefined, BapColorByTransmissionQueryArgs>({
-      query: ({ url }) => ({ url }),
+      query: ({ transmissionId }) => ({ url: getUrl(`/qwe/${transmissionId}`) }),
    }),

and

  } = useGetColorByTransmissionQuery(
-    { url: getUrl(`/qwe/${transmissionId}`), transmissionId },
+    { transmissionId },
    { skip: !transmissionId },
  );

On top of that, if your getUrl function is something like getUrl = (url) => process.env.BASE + url, you are further missing the point of baseQuery and should probably not be using that getUrl function.
In that case,

const bapApi = createApi({
  reducerPath: 'bapApi',
-  baseQuery: fetchBaseQuery(),
+ baseQuery: fetchBaseQuery({ baseUrl: process.env.BASE  })

and

  endpoints: builder => ({
    getColorByTransmission: builder.query<BapImages | undefined, BapColorByTransmissionQueryArgs>({
-      query: ({ transmissionId }) => ({ url: getUrl(`/qwe/${transmissionId}`) }),
+      query: ({ transmissionId }) => ({ url: `/qwe/${transmissionId}` }),
    }),

which you can even more simplify for GET requests as

  endpoints: builder => ({
    getColorByTransmission: builder.query<BapImages | undefined, BapColorByTransmissionQueryArgs>({
-      query: ({ transmissionId }) => ({ url: `/qwe/${transmissionId}` }),
+      query: ({ transmissionId }) => `/qwe/${transmissionId}`,
    }),

(Generally, please watch this video course to understand query and baseQuery)

Also, your

const bapApi = createApi({
  reducerPath: 'bapApi',

also makes me a bit afraid, because usually you should have one single api in your whole application.

I'm gonna be honest: I know these pattern from some YouTube tutorials, and those tutorials are unfortunately not the best quality and show very weird (anti-)patterns.

Maybe before you go deeper into your current discussion, could I motivate you somehow to spend some more time in our documentation? I'm sure when I can spot these problems that quickly, they're not gonna be the only ones.


On the topic: We recommend against copying data from the query cache into your own reducers, because it's very common that it might get stale there and you might miss updates, and your data might stay around even when it is unused and should be cache-collected. I would recommend using the RTK Query cache, maybe with a layer of selectors on top - see deriving data with selectors.

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

2 participants