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

Preact doesn't work with react-frame-component #2653

Closed
churichard opened this issue Jul 28, 2020 · 4 comments · Fixed by #3896
Closed

Preact doesn't work with react-frame-component #2653

churichard opened this issue Jul 28, 2020 · 4 comments · Fixed by #3896

Comments

@churichard
Copy link

churichard commented Jul 28, 2020

I'm trying to use Preact with react-frame-component and running into some issues. Specifically, it seems like when the iframe is unmounted it throws an error: Cannot read property 'parentNode' of undefined.

Some additional notes: I think the error is related to how Preact handles portals. The error doesn't occur when I comment out the createPortal call on https://github.com/ryanseddon/react-frame-component/blob/9f8f06e1d3fc40da7122f0a57c62f7dec306e6cb/src/Frame.jsx#L109. By default, this.props.head is null, and Preact doesn't seem to like that.

Reproduction

Here's a simple CodeSandbox that demonstrates the issue: https://codesandbox.io/s/preact-react-frame-component-y96k8?file=/app.js

Basically when one of the list items is selected, it'll become wrapped in a Frame component and colored red. However, it throws an error when you try to select a different item.

Notice that it works fine if there are no Frames, and it also works fine if you use React as well.

Steps to reproduce

Click on "Two".

Expected Behavior

"Two" should be wrapped in an iframe and colored red.

Actual Behavior

An error is thrown, saying Cannot read property 'parentNode' of undefined.

@lucasprag
Copy link

lucasprag commented Jan 31, 2023

I'm facing the same problem. I would definitely spend the time and create a PR to react-frame-component if I have some guidance of possible things to look for and if I'm able to move forward with it.

So far I was able to check that this.props.head is always null for me inside https://github.com/ryanseddon/react-frame-component/blob/9f8f06e1d3fc40da7122f0a57c62f7dec306e6cb/src/Frame.jsx#L109 regardless if I'm using Frame with either React or Preact. React is ok with it, but Preact isn't 😥

@andrewpye
Copy link

Not sure if it's an ideal solution, but I've found that passing an empty Fragment as the Frame component's head prop prevents the error from being thrown:

<Frame head={<Fragment></Fragment>}>
  <div>{title}</div>
</Frame>

I've forked the CodeSandbox here which shows the demo app working after making the update:

preact-with-frame-component

@developit
Copy link
Member

Hmm - we might actually just be able to remove the condition that handles null portals here:

if (props._vnode) {

That would make it so that createPortal(null, x) would create a Preact root and render null into it (which is valid, though not strictly necessary). It will mean that empty portals will have non-zero cost, but that cost will be negligible. Plus it'd make compat smaller!

/cc @JoviDeCroock can you think of any reason we wouldn't want to do this?

@JoviDeCroock
Copy link
Member

JoviDeCroock commented Feb 15, 2023

@developit that should be possible, just would need a change to how we do the condition for unmounting.

EDIT: tests seem to run correctly #3896

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

Successfully merging a pull request may close this issue.

5 participants