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: fix preact compat #268

Closed
wants to merge 1 commit into from

Conversation

nolanlawson
Copy link

fixes #254

I'm not sure why Preact is subtly different from React, but not caching this.parent fixes the error. I don't think there's any performance hit for not caching it (container.parentElement should be a cheap operation), so it seems worth it to get compatibility with Preact.

You can test this by using this branch and running the storybook. Before this fix, there's an error in the console; after this fix, there isn't.

@EtienneLem
Copy link
Member

Nice find, thanks. Sorry for the long delay on PRs, I’ll be reviewing / merging them very soon.

@nolanlawson
Copy link
Author

No problem. BTW I just did a bit of extended testing on this, and I think we're still not preact-compatible. There seem to be some bugs with switching categories. Maybe it's best to just put preact compatibility on the back burner, since both inferno and react-lite work.

@nolanlawson nolanlawson closed this Feb 5, 2019
@EtienneLem
Copy link
Member

Roger that. I’m currently working on a Preact project myself, so I don’t mind giving it a go once the other PRs have been sorted out.

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.

Preact compatibility: error on undefined parent in memoizeSize()
2 participants