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

Only schedule update if prop value has changed #224

Merged
merged 2 commits into from
Mar 24, 2021
Merged

Only schedule update if prop value has changed #224

merged 2 commits into from
Mar 24, 2021

Conversation

micahjon
Copy link
Contributor

@micahjon micahjon commented Feb 1, 2021

Add a check to reflectiveProp that gives it similar behavior to useState and attributeChangedCallback: an update is only scheduled if the prop's value has changed.

Fixes #192

Fixes Typescript compilation error due to unsubscribe property
not existing on ContextDetail when a context is used without a provider.
@matthewp
Copy link
Owner

Looks good, would you mind rebasing with master to see if the tests pass? Sorry again for missing this.

@micahjon
Copy link
Contributor Author

Hey @matthewp, no worries, looking forward to getting this wrapped up.

I'm fairly new to rebasing and could use some guidance on this one. As far as I can tell, my branch is up-to-date and only one commit (this PR) ahead of your master.
image

Also, I believe the Travis CI build will continue to fail as long as there's the unresolved Typescript warning (fixed in #223). If you merge that PR or fix it yourself, I think you'll find that the checks start passing.

@matthewp
Copy link
Owner

Ah, you're right!

@matthewp
Copy link
Owner

Ok, now a rebase. From your branch: git rebase master && git push -f origin only-update-if-prop-value-changed

Add a check to reflectiveProp that gives it similar behavior to useState
and attributeChangedCallback: an update is only scheduled if the prop's
value has changed.
@micahjon
Copy link
Contributor Author

Cool, thanks @matthewp, I think we're good to go

@matthewp matthewp merged commit 61744a5 into matthewp:master Mar 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Why is Component function called when a property value is set to itself?
2 participants