Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix theme providers not re-rendering their children #645

Merged
merged 1 commit into from
Dec 21, 2018

Conversation

troch
Copy link
Contributor

@troch troch commented Dec 14, 2018

Description

shouldComponentUpdate on theme providers prevent them to re-render their children if the provided theme hasn't changed. I understand it might have been added to prevent re-rendering all styled fela components if the theme hasn't changed, but React context uses reference identity to determine when to re-render. After checking, create-inferno-context and preact-context also have the same behaviour.

Checklist

Quality Assurance

You can also run yarn run check to run all 4 commands at once.

  • The code was formatted using Prettier (yarn run format)
  • The code has no linting errors (yarn run lint)
  • All tests are passing (yarn run test)
  • There are no flow-type errors (yarn run flow)

Changes

If one of the following checks doesn't make sense due to the type of PR, just check them.

  • Tests have been added/updated
  • Documentation has been added/updated
  • My changes have proper flow-types

@robinweser
Copy link
Owner

Would you consider it a patch release? Seems to me that nothing breaks as the new Context API supports that check out of the box.

@troch
Copy link
Contributor Author

troch commented Dec 16, 2018

Yep, patch

@troch
Copy link
Contributor Author

troch commented Dec 19, 2018

Anything you want me to change on this, or do you have any ETA on publishing?

@troch
Copy link
Contributor Author

troch commented Dec 19, 2018

#647 would allow me to have a workaround in the mean time

Copy link
Collaborator

@TxHawks TxHawks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine to me

@robinweser robinweser merged commit c8dbd5f into robinweser:master Dec 21, 2018
@troch troch deleted the theme-provider-fix branch December 21, 2018 13:32
@troch
Copy link
Contributor Author

troch commented Dec 21, 2018

Thanks a lot for merging and releasing 🙇 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants