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

Support React.memo #16

Open
billyjanitsch opened this issue Nov 23, 2018 · 5 comments · May be fixed by #19
Open

Support React.memo #16

billyjanitsch opened this issue Nov 23, 2018 · 5 comments · May be fixed by #19

Comments

@billyjanitsch
Copy link
Contributor

Currently, concordance crashes while trying to encode components wrapped in React.memo. These components are objects rather than functions or classes, which I suspect concordance doesn't know how to handle.

@novemberborn
Copy link
Member

Could you share a crash report and reproduction?

I'll definitely want to support this. Perhaps you're willing to try your hand at it? 😉 Hopefully it doesn't require a breaking change in AVA…

@billyjanitsch
Copy link
Contributor Author

See #17 for a repro and trace. I won't have time to dig into a fix for awhile but I'm happy to try if no one's gotten to it by then. 🙂

@novemberborn novemberborn linked a pull request Jan 12, 2019 that will close this issue
@novemberborn
Copy link
Member

Thanks @billyjanitsch. Could you have a look at #19 and see if my changes are sensible? For instance the ⍝ character, and how if you apply React.memo() twice to the same component, and then compare <first /> with <second /> they're considered equal?

@novemberborn
Copy link
Member

Also this new code does support existing serializations, so while we have to bump the snapshot version in AVA itself we can retain compatibility with v2 so we don't need to ship a new major version of AVA.

@novemberborn
Copy link
Member

@billyjanitsch would be great if you could have a look at #19, thanks 😄

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 a pull request may close this issue.

2 participants