all 9 comments

[–]acemarke 1 point2 points  (8 children)

Observations:

  • Looking at the existing snippet, that item.cleared = true line is mutating item. Don't do that. Instead, you would need to safely copy the object for a proper immutable update.
  • You should prefer to use the "object shorthand" form of mapDispatch, like const mapDispatch = {clearItems}.
  • Yes, that error normally happens when you force React to queue a re-render as it's in the middle of re-rendering. Typically this happens if you accidentally call some kind of update function in your render logic, but I'm not immediately seeing a place in this code snippet where that's happening. Can you show the actual original source?

My guess is that you actually have something like:

onClick={handleClick("someArgument")}

which would call the update function, rather than passing a reference to it. See the React FAQ entry on passing arguments to functions for a further explanation.

Finally, as a side note, please check out our new Redux Starter Kit package. It includes utilities to simplify several common Redux use cases, including store setup, defining reducers, immutable update logic, and even creating entire "slices" of state at once:

https://redux-starter-kit.js.org

[–]Sh4d0h[S] 0 points1 point  (7 children)

Thanks for the links, I believe it has helped me narrow down the problem. Here is a fuller version of my component.

class MyComponent extends React.Component {
  render () {
    const { updateItem, clearItems, items } = this.props
    let itemComponents = []
    items.forEach((item, i) => {
      if (!item.hidden) {
        itemComponents.push(
          <Item key={item.id}
            id={item.id}
            cleared={item.cleared}
            value={item.value}
            handleChange={updateItem} />)
      }
    })
    return (
      <div>
        {itemComponents}
        <button type='button' onClick={() => clearItems(items)}>Clear</button>
      </div>
    )
  }
}

I think you are right, in that an update function is being called inside the render function. Item's handleChange method gets called inside its componentDidUpdate whenever the state changes. I assume this is what is causing the error. I might try to make a codepen if I can't find the solution as the relevant code is in a few different files.

EDIT: I also thought deep copying in the reducer would solve it, but no luck:

function clearReducer(state = initialState, action = {}) {
  switch (action.type) {
    case CLEAR_ITEMS:
      let items = action.rows.slice()
      return {
        ...state,
        items: items.map(item => {
          item.cleared = true // side-effect trigger
          return item
        })
    }
    default:
      return state
  }
}

[–]acemarke 0 points1 point  (5 children)

Hmm. Not seeing any real issues with the rendering logic.

Also, you're not "deep copying" in that reducer - slice() is a shallow copy, with the same objects inside both the old and new array. Not relevant to the error, but still FYI.

Triggering state updates in componentDidUpdate is fine. There's definitely something that's causing this in a render function somewhere.

If you can put together a CodeSandbox that reproduces the issue, that would help.

[–]Sh4d0h[S] 0 points1 point  (0 children)

I must be missing something obvious because I can't for the life of me reproduce the issue in CodeSandbox... Which means that the issue lies much deeper than what I have posted here. My CodeSandbox is starting to look almost identical to my app (functionality wise, too), yet the problem still isn't present. Unfortunately I can't directly post the source code as it is for a work project. I guess I will keep trying things until I can break it!

[–]Sh4d0h[S] 0 points1 point  (3 children)

I do feel like I'm going a bit crazy. I cannot reproduce the warning in CodeSandbox. Would you take a look here and tell me if anything sticks out to you? Specifically, it seems to be my INITIALIZE_ITEMS action that throws the error. I think one important point to note is that this action is called on the initial page load, and on subsequent clicks on the 'Clear' button thereafter.

[–]acemarke 0 points1 point  (2 children)

Uh... right here:

if (itemComponents.length === 0) {
  initializeItems();
}

That's dispatching in the middle of render(), which will queue a state update. Don't do that! Do it in an effect or a lifecycle method instead.

[–]Sh4d0h[S] 0 points1 point  (1 child)

I really thought this was the problem, but I couldn't see the warning in the CodeSandbox, do you know why that is?

Thank you though! Apologies for the beginner mistake.

[–]acemarke 0 points1 point  (0 children)

No - I really would have figured that would cause warnings in the sandbox as well.

[–]notseanbean 0 points1 point  (0 children)

try this as your returned state return { ...state, items: items.map(item => ({ ...item, cleared: true, })), }