draggable component does not get rerendered until clicked when not using inline function

[alukach]

Bug or feature request?

Bug

Similar to #156, however I'm not using MobX.

My state object is updated after move but the change is not represented in the view until I click on an element (i.e. after dropping the moved element, it appears that it has returned to its source location despite the state showing otherwise).

This only occurs when I'm not using an inline function.

Bad (Same result when using a non-arrow function):

export default class TemplateEditor extends React.Component<Props, State> {

  public renderTemplateElements = (provided: DroppableProvided) => (
    <TemplateElementList
      innerRef={provided.innerRef}
      {...provided.droppableProps}
    >
      {this.props.template.layout.map((element, i) => (
        <TemplateElementComponent
          key={i}
          index={i}
          id={i.toString()}
          element={element}
        />
      ))}
      {provided.placeholder}
    </TemplateElementList>
  );

  public render() {
    return (
      <DragDropContext onDragEnd={this.onDragEnd}>
        <Container>
          <div className="m-2">Visual Editor</div>
          <Droppable droppableId="someUniqueId">
            {this.renderTemplateElements}
          </Droppable>
        </Container>
      </DragDropContext>
    );
  }
}

Good:

  public render() {
    return (
      <DragDropContext onDragEnd={this.onDragEnd}>
        <Container>
          <div className="m-2">Visual Editor</div>
          <Droppable droppableId="someUniqueId">
            {(provided: DroppableProvided) => (
              <TemplateElementList
                innerRef={provided.innerRef}
                {...provided.droppableProps}
              >
                {this.props.template.layout.map((element, i) => (
                  <TemplateElementComponent
                    key={i}
                    index={i}
                    id={i.toString()}
                    element={element}
                  />
                ))}
                {provided.placeholder}
              </TemplateElementList>
            )}
          </Droppable>
        </Container>
      </DragDropContext>
    );
  }

What version of React are you using?

16.4.7

What version of react-beautiful-dnd are you running?

7.1.2

[garkin]

I got this issue too. Sad reason is Redux {pure: true} HOC being used: https://github.com/atlassian/react-beautiful-dnd/blob/6f71b45c13aab36807f9da952437a5d63d52d696/src/view/droppable/connected-droppable.js#L140

[garkin]

I think this is a bug. There is no need to check mutations in case we always send mutated children={new lambda()} prop. It's a CPU waste. You should not assume function returns same result if function is the same. Dynamic composition is a whole reason to have a function in the first place. Not to mention about nesting hell and ill patterns with lambdas in render.

Related caution from the official docs: https://reactjs.org/docs/render-props.html#be-careful-when-using-render-props-with-reactpurecomponent

[alexreardon]

Fasinating! We have a test to ensure that parent renders are passed through:

https://github.com/atlassian/react-beautiful-dnd/blob/master/test/unit/view/connected-droppable/child-render-behaviour.spec.js

But the test is not exercising the case where a Droppable function is an instance property and not an inline function. This is worth investigating.

We should be allowing all parent renders through

[alexreardon]

Also, please confirm the issue is still occurring on the latest version. 7.x is two majors behind the current supported version

[alexreardon]

Thanks!

[garkin]

I'm getting it in "react-beautiful-dnd": "^9.0.2"

[alexreardon]

Any thoughts on this @markerikson?

[markerikson]

Mmm... not specifically, other than connect() does default to ensuring that your wrapped component only re-renders when either its data from the Redux store or the props passed to the wrapper component have actually changed.

[alexreardon]

@markerikson It looks like using an arrow function changes the children props (a new reference every time)

At least, that is what I thought until I set areOwnPropsEqual to () => false and every component re-rendered every time...

[markerikson]

So just to check: you're rendering <SomeComponent constantProp={123}>{constantChild}</SomeComponent> ? In that case, yeah, if that component isn't extracting new values in its mapState, then connect will skip re-rendering it. Passing in a new arrow function as that child would be a new prop, so it would re-render.

[alukach]

Btw, I mispoke, I am using v9.0.2

[alexreardon]

@markerikson here is what I am seeing:

• all connected components are using the default areOwnPropsEqual (`shallowEqual)
• technically every component has a new children on each render (new arrow function) <SomeComponent constantProp={123}>{() => <Child />}</SomeComponent>
• only the ones that has their mapState updated render

this is the behaviour I want, but it is a little strange as I would think that each component's areOwnPropsEqual check would fail. If i set areOwnPropsEqual to () => false then every component re-renders on every update

I am confused by this behaviour

[markerikson]

Hmm. That does sound odd. Do you have a CodeSandbox or other project that repros this issue?

[alexreardon]

I can make one tomorrow On Sun, 28 Oct 2018 at 5:48 am, Mark Erikson notifications@github.com wrote:

Hmm. That does sound odd. Do you have a CodeSandbox or other project that repros this issue?

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/atlassian/react-beautiful-dnd/issues/854#issuecomment-433645668, or mute the thread https://github.com/notifications/unsubscribe-auth/ACFN7eun8bNBac3_MOLyUbsRnoeGDySVks5upKppgaJpZM4XYdUf .

[markerikson]

@alexreardon : Did you ever dig into this issue further?

[dandelany]

Just wanted to ping this issue again, as I ran into it today ("react-beautiful-dnd": "10.0.3") and spent some time confusedly debugging before finding this ticket. Relatively easy to work around it using an arrow function, but it's hard to track down & def could cause confusion for users.

[alexreardon]

For sure, this looks like a deep react-redux issue. I am hoping it will just go away when we move to hooks #871

[alexreardon]

It could be worth calling out in the docs

[alexreardon]

PR's welcome!

[markerikson]

FWIW, I don't think I ever did see any kind of a repro project or test that I could look at with this.

[alexreardon]

I stopped digging after a while. I think I got distracted.

My understanding: if you use a render props function then your children prop is always different, which has some flow-on effects.

[alexreardon]

Can somebody check to see if this issue still exists on 12.0.0-alpha.7?

[alexreardon]

I'll close this for now as I have not heard any updates on it. Please feel free to comment on this and we can reopen it if it is still an issue for people

[robbertbrak]

Still an issue. I ran into this today and had to waste some time debugging this before I found this ticket. How about mentioning this in the "Common setup issues"?

[SpringsTea]

Still an issue on 13...