addClass and removeClass implementation should be consistent across all components

[moog16]

There are currently 2 different implementations of removeClass:

      removeClass: (className: string) => {
        const classList = new Set(this.state.classList);
        classList.delete(className);
        this.setState({classList});
      },

AND

      removeClass: (className: string) => {
        const {classList} = this.state;
        classList.delete(className);
        this.setState({classList});
      },

The difference being creating a new Set(). We should not be altering state directly, which is why we create new Set. addClass should also follow this pattern.

Related https://github.com/material-components/material-components-web-react/pull/792/files#r271249191

[ghost]

To avoid mutating the current state and possible async issues I would write it like:

removeClass: (className: string) => {
  this.setState((prevState) => {
    const classList = new Set(prevState.classList);
    classList.delete(className);
    return {classList};
  });
}

Same for addClass, like you mentioned.

[moog16]

Ya we have that in some places. What is the benefit of that syntax vs

removeClass: (className: string) => {
        const classList = new Set(this.state.classList);
        classList.delete(className);
        this.setState({classList});
      },

[ghost]

It avoids the problems that arise from how react batches state updates. For example if removeClass is called twice in a row only the last call would have an affect, since it would override the first.

In my example, the previous state is taken into consideration. A better name for that variable in this scenario could be currentState.

[moog16]

FYI - I just ran into this issue in the enhanced select (which I'm currently implementing). It happens on the foundation.handleBlur call, where removeClass is called twice. This fixed it!