Skip to content

Draft: Dispatch fetch actions only when actually needed

Axel Bocciarelli requested to merge reduce-console-noise into master

Opening as draft to start a discussion.

I'm trying to get rid of these console messages to reduce the noise in development and when running tests:

image

  • The line that logs this is here: https://gitlab.esrf.fr/ui/redux-provider/-/blob/master/src/Actions.js#L77

  • It gets called when the promise function of a namespace's async action returns a falsy value. For instance in providers/hardward.js on line 102:

    promise: (payload, state) => {
      if (
        payload.first &&
        (state.ns_hardware.default.fetched ||
          state.ns_hardware.default.fetching)
      ) {
        return undefined;
      }
      return RestService.get('/hardware');
    }

The purpose of this condition seems to be to avoid fetching hardware objects if they're already fetched or being fetched. To get this behaviour, one has to pass { first: true } as payload to the FETCH_HARDWARE_default action. For instance, in /connect/hardware/Hardware.js:

const mapDispatchToProps = (dispatch, own) => {
  const { hardware, groups } = own.providers.hardware;
  return {
    actions: {
      fetch: payload => hardware.fetch({ first: true }),
      fetchGroups: payload => groups.fetch({ first: true })
    }
  };
};

While it would be easy to just remove the console.log, I wonder if the current behaviour of "skipping" an async action like this is the right way to go. I'm thinking that it is perhaps more the Redux Way ™️ to avoid dispatching the action in the first place?

This is what I've attempted to do in this PR. Basically, I'm replicating the promise function logic in /components/hardware/Hardware. The clear downside is that it takes a lot more boilerplate, since one has to provide the fetched and fetching props to the component. Here it wasn't too bad since fetched was already provided, but in other places like Synoptic, it's a bit more work. Also, it means duplicating the same logic in multiple places, which is annoying.

With suspense and a resource fetching/caching library like rest-hooks, this kind of problem would not exist, so perhaps it is not worth trying to find a complex solution in the current architecture and better to comment out the console.log in redux-provider. Let me know what you think.

Edited by Axel Bocciarelli

Merge request reports

Loading