adobe-photoshop / spaces-design

Adobe Photoshop Design Space
http://adobe-photoshop.github.io/
Other
853 stars 74 forks source link

Size, Position and Radius shouldComponentUpdate are broken #3554

Closed iwehrman closed 8 years ago

iwehrman commented 8 years ago

All return this.state !== nextState. This does not do what it seems like it should do. In particular, if the same object is used for state then this will always return true, even if the properties have changed. And, if a new object is used for state then this will always return false even if all the properties are the same. I don't know of any particular bug that this causes, but at some point it almost certainly will cause one!

iwehrman commented 8 years ago

Tentatively over to @volfied since I think he wrote these lines in the first place. (My git blame reading could be wrong, though.)

baaygun commented 8 years ago

@iwehrman I've written it this way, because we calculate the state out of props at componentWillReceiveProps. Both components do not use any of the props to render, and rely on state to render everything. I'll think about it a bit more, but it was a conscious decision on my part which I ran by React people.

iwehrman commented 8 years ago

I don't get it. Depending on the implementation of setState this will always return true or it will always return false. Remember that state is a mutable object. This would be a great implementation if state were an Immutable object.

chadrolfs commented 8 years ago

If this is a case where it would show up to users as a case where the object appearance on canvas is separate from it's location (coordinates/highlight) then I've hit this recently a few times but haven't been able to reliably repro. Taki has hit it as well but can't reliably repro. Not sure if this is would cause this or if that's a separate issue.

iwehrman commented 8 years ago

This would only manifest as incorrect values in the Size, Position or Radius controls in the transform panel.

iwehrman commented 8 years ago

Fixed by #3573.