react-refetch icon indicating copy to clipboard operation
react-refetch copied to clipboard

skip/disable/remove fetch config conditionally

Open slorber opened this issue 6 years ago • 4 comments

Hi,

What I want to do is pretty simple and I thought it would be supported but I can't find such an option so far.

I have an input text on which I'd like to provide autocomplete. But I don't want to trigger requests unless the text size is > 2.

I've tried the following:

const ConnectedCompanyField = connect(({ value }) => {
  if (value.length > 2) {
    return {
      fieldValuesFetch: {
        comparison: value,
        value: () => searchField(value, ProviderLabel),
      },
    };
  }
})(CompanyField);

Even if the fieldValuesFetch is not returned when text is small, the former fieldValuesFetch remains injected into the component. It looks like a bug to me, but maybe it's intended?

I think it would be easier to provide an ability to "skip" / "ignore" a fetch conditionnally.

Apollo provides an API similar to this:

const ConnectedCompanyField = connect(({ value }) => {
      fieldValuesFetch: {
        comparison: value,
        value: () => searchField(value, ProviderLabel),
       skip: value.length <= 2,
      },
})(CompanyField);

slorber avatar Sep 11 '18 17:09 slorber

Even if the fieldValuesFetch is not returned when text is small, the former fieldValuesFetch remains injected into the component

What do you mean by "remains injected into the component"? If I'm understanding correctly, isn't that what you'd want; otherwise, there would be nothing for the component to render.

I think it would be easier to provide an ability to "skip" / "ignore" a fetch conditionnally.

Probably the easiest thing to do would be to set comparison to something like value.length > 2 && value. This will make it false unless it's longer than 2 characters and will skip fetching (because false === false, not that false is has some special meaning here).

ryanbrainard avatar Sep 12 '18 05:09 ryanbrainard

@ryanbrainard

  • initially my component does not receive any fieldValuesFetch (PromiseState) prop
  • then I type text, and it receives fieldValuesFetch as soon as there is more than 3 chars
  • then I erase the text, and make the text empty, and the component still receive the fieldValuesFetch which actually contains the result of the last text that triggered the fetch

I'd like no NOT receive that prop when I make the text input empty again, because otherwise my autocomplete dropdown keeps showing former results when the input is empty. It looks to me the lib should do state cleanup when it detects that a fetch config has just been removed.

I'd like to never receive the fieldValuesFetch prop when the input text length <= 2. Currently, this is not consistant and depends on former fetch results.

As expected, the following does not really do what I want:

const ConnectedCompanyField = connect(({ value }) => {
  return {
    fieldValuesFetch: {
      comparison: value.length > 2 && value,
      value: () => searchField(value, ProviderLabel),
    },
  };
})(CompanyField);

With this config the searchField promise API still fires, it's just it will fire only once as long as the comparison remains false, which can lead to something even more dangerous: results being queried for a small texts of 2 chars, and results not changing when the 2 letters do change because comparison remains constant

slorber avatar Sep 12 '18 07:09 slorber

Here is a basic code solution to add the behavior I want. I'd like to submit a proper PR if you agree with this behavior and the implementation:

        const arrayDiff = function(a,b) {
          return a.filter(function(i) {
            return b.indexOf(i) < 0;
          });
        };
        
        const omitKeys = function omitKeys(obj,keys) {
          const { x, ...copy } = obj;
          keys.forEach(key => delete copy[key]);
          return copy
        }
      refetchDataFromMappings(mappings) {
        mappings = coerceMappings(mappings)
        Object.keys(mappings).forEach(prop => {
          /// ... nothing here, keep the original code
        })
       
        // Do state cleanup on mapping removal
        const removedMappingProps = arrayDiff(Object.keys(this.state.mappings),Object.keys(mappings));
        // clear refresh timeouts before removal
        if ( removedMappingProps.length ) {
          removedMappingProps.forEach(prop => {
            if (this.state.refreshTimeouts[prop]) {
              window.clearTimeout(this.state.refreshTimeouts[prop]);
            }
          })
          // remove prop mapping from state
          this.setState({
            mappings: omitKeys(this.state.mappings,removedMappingProps),
            data: omitKeys(this.state.data,removedMappingProps),
            startedAts: omitKeys(this.state.startedAts,removedMappingProps),
            refreshTimeouts: omitKeys(this.state.refreshTimeouts,removedMappingProps),
          });
        }

      }

slorber avatar Sep 12 '18 07:09 slorber

Alternatively, could you just control this with a conditional value. I haven't tried it, but I think this would clear the previous PromiseState, or at least with an undefined value and skip the fetch:

comparison: value,
value: value.length < 2 ? undefined : () => searchField(value, ProviderLabel),

I'd rather not change the internals of the library for this since. Perhaps we could have some utility to help make this easier, but I don't think previously set state should "automagically" be cleared.

ryanbrainard avatar Sep 17 '18 17:09 ryanbrainard