Update useDimensionsChanged hook so that it causes re-render

[timmydoza]

Contributing to Twilio

All third party contributors acknowledge that any contributions they provide will be made under the same open source license that the open source project is provided under.

• I acknowledge that all my contributions will be made under the project's license.

Pull Request Details

JIRA link(s):

• N/A

Description

This pull request contains a bugfix submitted by Github user @tomhicks. I've pulled his changes into a new PR so that the Circle CI tests can run on it. Here is the original PR: #376

This PR fixes an issue found in the useDimensionsChanged hook, where the handleDimensionsChanged function would set the same object reference as the one already stored in the hook's state. This means that component that use this hook would not re-render when a track's dimensions have changed.

handleDimensionsChanged now returns a new object reference when a track's dimensions have changed, which will correctly cause components to re-render.

Burndown

Before review

• Updated CHANGELOG.md if necessary
• Added unit tests if necessary
• Updated affected documentation
• Verified locally with npm test
• Manually sanity tested running locally
• Included screenshot as PR comment (if needed)
• Ready for review

Before merge

• Got one or more +1s
• Re-tested if necessary

[mhuynh5757]

Do we still need the ? postfix operator on track here? We could also do something like setDimensions({ ...track?.dimensions }) so we get all of the properties like we used to, just in case something relied on any other track properties.

[tomhicks]

If you need the null coaslescing, the types are wrong, so change those rather than reinstating what appears to be a redundant operator.

You're probably right about cloning the track dimensions to keep other properties, although you might have other deeply-nested objects that will suffer from the 'same reference' problem.

It might be better to specify exactly the return shape of this hook, rather than have it infer it from elsewhere.

[timmydoza]

Do we still need the ? postfix operator on track here?

No - it isn't needed inside the if (track) { block.

if (track) {
  // track will never be undefined in here
}

We could also do something like setDimensions({ ...track?.dimensions }) so we get all of the properties like we used to, just in case something relied on any other track properties.

We could, but width and height are the only properties on this object, so I think the code is fine as is. https://media.twiliocdn.com/sdk/js/video/releases/2.10.0/docs/VideoTrack.html#.Dimensions