reduxjs / redux-toolkit

The official, opinionated, batteries-included toolset for efficient Redux development
https://redux-toolkit.js.org
MIT License
10.63k stars 1.15k forks source link

Redux store vs Redux API queries #4283

Open MaximusFT opened 5 months ago

MaximusFT commented 5 months ago

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

phryneas commented 5 months ago

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.