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

skip/disable/remove fetch config conditionally #215

Open
slorber opened this issue Sep 11, 2018 · 4 comments
Open

skip/disable/remove fetch config conditionally #215

slorber opened this issue Sep 11, 2018 · 4 comments

Comments

@slorber
Copy link

slorber commented Sep 11, 2018

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);
@ryanbrainard
Copy link
Contributor

ryanbrainard commented Sep 12, 2018

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).

@slorber
Copy link
Author

slorber commented Sep 12, 2018

@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
Copy link
Author

slorber commented Sep 12, 2018

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),
          });
        }

      }

@ryanbrainard
Copy link
Contributor

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.

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