Unable to convert setState(function) to hooks

[aeciorc]

I'm unable to convert to class to functions when I use setState with a function argument. For example:

this.setState(prevState => ({
      profile: {
        ...prevState.profile,
        roles: [
          ...prevState.profile.roles,
          {
            title: "",
            link: "",
          }
        ]
      }
    }));
  };

The expected output should be:

setProfile({ ...profile, roles:[...profile.roles, {title: "", link: "" }]})

But instead I get an error: ""Cannot read property 'forEach' of undefined".

[borislit]

@aeciorc Thanks for the report! Ill have a look ASAP, unless you wanna give it a go yourself :) Thanks

[aeciorc]

Sure, I'll have a try at it

[borislit]

@aeciorc just a quick update. I've been researching the problem. Glean currently doesnt support passing a function to a state hook. useState does support passing a function to its setter, but the problem here is that the object u are passing to this.setState can potentially contain multiple state updates and thus need to be inspected. It can be pretty complex since you can have multipe variations of that. Ill try to come up with a simple solution at first that will at least cover your case

[aeciorc]

Awesome, @borislit, thanks!

[borislit]

@aeciorc just a short update. IM currently on vacation. Ill have a fix in a week or so when im back

[aeciorc]

Sounds good @borislit, thanks. Meantime I've created a small PR for another small issue I was having: https://github.com/wix/vscode-glean/pull/85

[borislit]

@aeciorc short update. I've done plenty of progress on the solution. I think ill have it ready for review next week. I have managed to handle most cases (turns out there are several). Currently, the biggest issue is the following use case: this.setState(prev =>({ foo: prev.foo, bar: prev.bar })) The issue here is that I'll need to rewrite references to prev.foo and prev.bar.

[aeciorc]

@borislit I see, it can get pretty complicated. Another thing I've noticed is that after the refactoring, we may end up with reference errors if a variable we're passing to setState isn't available on the outer scope. E.g:

fetchData = () => {
    fetch("/").then(r => {
      this.setState({ m: r});
    });
  };

would cause the result to have

const [m, setM] = useState(r);

I was thinking about working on a feature that initializes the state with null when that happens. What do you think about it?

[borislit]

@aeciorc Thats actually a bug. I've noticed it too and already have a fix for it in a branch im working on

[borislit]

@aeciorc Boom! It should fully support using functions to update the state now https://github.com/wix/vscode-glean/pull/86 Im releasing a new version now, that will include your PR and this. Check it out. Tell me what you think

[borislit]

Ok i see that there are still some issues. Ill push an additional fix

[borislit]

Ok i was too quick to rejoice. TravisCI is totally stuck so the release is delayed :(

[aeciorc]

ahaha shoot. Hmu if I there's anything I can do to help

[borislit]

@aeciorc The new version should be up! Check it out (4.18.0)

[aeciorc]

Hey @borislit , I tried it out, the functional setState conversion is working great! Noticed that this release added one bug though: If a state hasn't been initialized (either in the constructor or class property), the useState declaration won't be generated for it. E.g:

class Foo extends React.Component {
  lol = () => {
    this.setState({ done: true });
  };
  render() {}
}

becomes

const Foo = props => {
  const lol = useCallback(() => {
    setDone(true);
  });
};

Unrelated to the release, I also noticed a case for which converting to hooks might be very hard:

  handleCheckbox = type => {
  //type can be "checkboxA", "checkboxB", etc
    this.setState({ [type]: true });
  };

Since the value of "type" isn't know til runtime, the state would have to be an object: setBla({...bla,[type]:true}). But if somewhere else in the original code there was a this.setState for a specific checkbox type, like this.setState({checkboxA: true}), we'd end up two state hooks for the same class state lol

[borislit]

Hey @aeciorc ! So, I've pushed the fix for the first issue. Let me know. In terms of the second issue - I dont think this should be solved. Its sort of an anti pattern and I think there is no way for it to be expressed with hooks. I mean, you cant do something like set[type](). I mean you can, but in a totally hacky, terrible way.

[aeciorc]

I've tried it with a few test cases, seems to be working perfectly @borislit! Yes, can't disagree about it being a bad pattern, although it's useful when working with forms because you can give each input a name corresponding to its state and have a single handler onChange = event => this.setState({[event.target.name]:event.target.value}) Anyways, thanks for the release, it's definitely gonna help me save time in the future!