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

add __v and __o keys to the custom handler to support Preact #67

Merged

Conversation

dios-david
Copy link
Contributor

Hi there,

I'm using preact and I noticed this console warning in my browser:
react-fast-compare cannot handle circular refs

After a bit of investigation I found out that one of my dependencies is using react-fast-compare which would work fine with preact as well after a minor modification.
Instead of forking the project (e.g. preact-fast-compare) I thought it would be better to contribute to the project and make it work with both frameworks.

@developit
Copy link

developit commented Apr 16, 2020

Strong +1 from the Preact core team on this!

A suggestion: perhaps it would be worth ignoring all properties that have leading _? It'd be a more widespread change, but would also be a nice way to allow hiding properties from comparison that doesn't require defining them as non-enumerable.

@dios-david
Copy link
Contributor Author

dios-david commented Apr 17, 2020

Yeah that sounds reasonable, I'm happy to make those changes if needed. I leave the decision up to the maintainers. @chrisbolin

@chrisbolin
Copy link
Contributor

alright, current quick thoughts...

  • the code change would be minimal
  • the support change would be a little more. we'd have to make sure we support future versions of preact and adopt their release cadence
  • we should probably have tests for preact as well

@chrisbolin
Copy link
Contributor

oh also... i'm nervous about ignoring all _-prefixed properties: this could both be a regression for existing users AND cause new users some pain and confusion.

@chrisbolin chrisbolin marked this pull request as draft April 30, 2020 14:18
@chrisbolin chrisbolin added the enhancement New feature or request label Apr 30, 2020
@chrisbolin
Copy link
Contributor

just thinking ahead: when this is finished we should release as a minor version. I don't think it warrants a major release

@kale-stew kale-stew marked this pull request as ready for review May 5, 2020 17:44
@kale-stew kale-stew changed the base branch from master to preact-support May 5, 2020 17:44
@kale-stew kale-stew changed the base branch from preact-support to master May 5, 2020 17:46
@kale-stew kale-stew changed the base branch from master to preact-support May 5, 2020 17:48
@kale-stew
Copy link
Contributor

Merging this into a feature branch so we can move forward with polishing up new Preact tests separate from your work, @dios-david. Thanks for introducing this feature. 🙌

@kale-stew kale-stew merged commit ed6058a into FormidableLabs:preact-support May 5, 2020
@kale-stew kale-stew mentioned this pull request May 6, 2020
4 tasks
@kale-stew kale-stew mentioned this pull request May 8, 2020
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants