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

AutocompleteArrayInput wrong behavior on input changes #4724

Closed
edulecca opened this issue Apr 25, 2020 · 10 comments · Fixed by #5044
Closed

AutocompleteArrayInput wrong behavior on input changes #4724

edulecca opened this issue Apr 25, 2020 · 10 comments · Fixed by #5044
Labels

Comments

@edulecca
Copy link
Contributor

edulecca commented Apr 25, 2020

What you were expecting:
AutomcompleteArrayInput should not log any React warnings on component input value changes related to useMemo or useEffect hooks.

What happened instead:
Every time the AutomcompleteArrayInput gets a new value or some of them are deleted, so on every component's update, the console log 2 warning related to useMemo and useEffect

Warning: The final argument passed to useMemo changed size between renders. The order and size of this array must remain constant.

Previous: [collector, compulsive, returns, function (value) {
    return choices.find(function (choice) {
      return lodash_get__WEBPACK_IMPORTED_MODULE_3___default()(choice, optionValue) === value;
    });
  }]
Incoming: [collector, compulsive, function (value) {
    return choices.find(function (choice) {
      return lodash_get__WEBPACK_IMPORTED_MODULE_3___default()(choice, optionValue) === value;
    });
  }]
.......
Warning: The final argument passed to useEffect changed size between renders. The order and size of this array must remain constant.

Previous: [collector, compulsive, returns, function (eventOrValue) {
    var event = eventOrValue;
    var value = event.target ? event.target.value : eventOrValue;
    setFilterValue(value);

    if (setFilter) {
      setFilter(value);
    }
  }]
Incoming: [collector, compulsive, function (eventOrValue) {
    var event = eventOrValue;
    var value = event.target ? event.target.value : eventOrValue;
    setFilterValue(value);

    if (setFilter) {
      setFilter(value);
    }
  }]
.....

Steps to reproduce:
Ir order to reproduce the issue just import AutocompleteArrayInput and use it within a wrapper or just without it and passing directiley the choices with a constant.

Related code:
Just replace on examples/demo/src/visitors/SegmentsInput.tsx the <SelectArrayInput/> to <AutocompleteArrayInput/> and will get the warnings.

Other information:
There is a note on the component

    // We must reset the filter every time the value changes to ensure we
    // display at least some choices even if the input has a value.
    // Otherwise, it would only display the currently selected one and the user
    // would have to first clear the input before seeing any other choices
    useEffect(() => {
        handleFilterChange('');
    }, [...values, handleFilterChange]);``

couldnt figure it out if the "reset the filter" is afecting the hooks behaivor

Environment
node v13.12.0
ra-admin 3.4.2
OS OSX 10.15.4
chrome 81.0.4044.113

@wdencker
Copy link

wdencker commented May 3, 2020

I am getting these console errors as well. @edulecca Did you find a solution?

@edulecca
Copy link
Contributor Author

edulecca commented May 6, 2020

@wdencker no I don't

@djhi djhi changed the title AutocompleteArrayInput wrong behaivor on input changes AutocompleteArrayInput wrong behavior on input changes Jun 4, 2020
@djhi djhi added the bug label Jun 4, 2020
@djhi
Copy link
Collaborator

djhi commented Jun 4, 2020

Reproduced in the simple example. The <TagReferenceInput> component in the PostEdit.js file does log a lot of warnings

@helenwilliamson
Copy link
Contributor

Hey.

We're also seeing the same issue. Is there a fix being worked on for this (as it's causing a lot of noise in our tests).

Thanks,

@djhi
Copy link
Collaborator

djhi commented Jun 23, 2020

I don't think anyone tried to fix it yet. Feel free to open a pull request

@jonathanpopp
Copy link

Any update on a potential fix for this or workaround?

@helenwilliamson
Copy link
Contributor

helenwilliamson commented Jul 13, 2020

@djhi can you explain the comment on useEffect? It's not clear to me what behaviour it's trying to ensure.

I've replaced the SelectArrayInput in the demo app with AutocompleteArrayInput and the component appears to render correctly. Unfortunately the issue number mentioned in the test doesn't seem to exist:

@helenwilliamson
Copy link
Contributor

helenwilliamson commented Jul 13, 2020

Assuming the useEffect is required, i've made a PR which removes the warning and has the same behaviour. This uses length on values rather than spreading values, as this means there's a consistent sized array passed to useEffect (and the same thing is done with useMemo)

#5044

@helenwilliamson
Copy link
Contributor

@djhi are you happy with the fix I proposed?

@fzaninotto
Copy link
Member

Fixed by #5044

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants