instead of "function", can we make it in a constant file and export it @StevenLangbroek

I think returning early if a shallow comparison fails would be a breaking change, and incompatible with React's setState behavior. React docs discourage returning mutated state from the setState updater function (or passing a mutated state object to setState), but neither is prohibited. From the React docs:

setState() will always lead to a re-render unless shouldComponentUpdate() returns false.

An example demonstrating a successful state-mutation update is here.

My second concern is that React emits its own development-time console warnings for passing null to setState, and by returning silently we might prevent a developer from realizing they have made a mistake.

For both of those reasons, I think it might be best to eliminate the conditional early return altogether, and let React do its thing.

So in other words, I would think about rolling back ff77c96.

setState() will always lead to a re-render unless shouldComponentUpdate() returns false.

I think this is not true with react@16. I don't use this feature but returning null is supposed to prevent scheduling a re-render.

What we might need to introduce here is 2 distinct paths for objects & function styles. Because of the null issue we cannot combine both and use newState in following lines, it should be restructured to something like:

  setState(state: $Shape<State> | (State => $Shape<State>)): void {
    const prevState = this.state;
    let newState
    if (typeof state === 'function') {
      newState = state(prevState)
      
      if (newState === null) {
        return;
      }
    } else {
      newState = state

      if (process.env.NODE_ENV !== 'production' && newState === null) {
        console.warn('write a nice dev warning here')
      }
    }

    if (newState === prevState) {
      return
    }

    this.state = Object.assign({}, prevState, newState);
    this._listeners.forEach(fn => fn());
  }

Does it make sense to u?

Yeah I understand how to get there code-wise, I'd just like to have @jamiebuilds's input on what the desired behavior is.

Well, the API should mirror React's API as closely as it can. So I'd say we should just follow that path.

Yeah since this is already only conceptually similar to setState, I'm not convinced we should be following React to the letter vs doing the principally right thing. Their API tradeoffs aren't necessarily the same as those of a pure state manager. I'll update tomorrow, and hopefully get some time in to write those tests.

I want it to match the React behaviour as closely as possible. Refactoring component state into container state should be near seamless

Wow my mistake! I was operating under the impression the Subscribe component's onUpdate() subscription callback was receiving the updated state and passing it to setState(). I should have read more closely. 🙂

@StevenLangbroek Given the above discussion I concede it makes sense to have custom error reporting and perhaps disregard what React does or does not report as a mistake.

However, the documentation for Unstated is currently written without any specification for setState(). The implication is that setState should behave the same way you would expect already, having worked with React. That means:

• null prevents a re-render
• But equal object identity does not prevent a re-render.

If there's an intent to change that, it should be documented well, and also be considered a breaking change which would merit a new major version.

@jamiebuilds I missed your reply, but I guess I should have just let your comment sit. 😄 I appreciate that sentiment - it's my favorite thing about this library.

  if (process.env.NODE_ENV !== 'production' && newState === null) {
        console.warn('write a nice dev warning here')
      }

Hi @StevenLangbroek , can we make one constant file and manage this kind of string production ? because in code we are using this kind of thing repeatedly and so instead of doing repetitively we can handle it in one file use.

Is this even necessary? Or should I just return?

assignment indicates that it changes something, I think return would be more appropriate here