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

Calling close method in React #576

Closed
mihkeleidast opened this issue Apr 25, 2018 · 10 comments
Closed

Calling close method in React #576

mihkeleidast opened this issue Apr 25, 2018 · 10 comments

Comments

@mihkeleidast
Copy link

Calling the close method on an iframe inside a React component fails with an uncaught DOMException:

Uncaught DOMException: Failed to execute 'removeChild' on 'Node': The node to be removed is not a child of this node.

This happens due to this line: https://github.com/davidjbradshaw/iframe-resizer/blob/master/src/iframeResizer.js#L552

The exception happes because in React, React itself handles the addition/removal of DOM elements and has already removed the element by the time iframe-resizer tries to to removeChild. But I would still like it to delete the reference to the iframe, otherwise i get a "Iframe not found" warning on every resize.

My react-iframe-resizer component: https://gist.github.com/risker/14360929f92c307feb06b146a8c379eb

The removal of the iframe element could be put behind an option flag, so the library would play better with React.

@davidjbradshaw
Copy link
Owner

Have you tried this?

https://github.com/brudil/react-iframe-resizer

@mihkeleidast
Copy link
Author

mihkeleidast commented Apr 26, 2018

Tried updating my componentDidMount/componentWillUnmount methods to the same as in react-iframe-resizer, but that fails with Uncaught TypeError: this.iframeResizer.close is not a function. Not sure if I'm doing something wrong, but that does not seem to work at all. iframe-resizer seems to return an array of iframe elements, not the instance.

https://github.com/zeroasterisk/react-iframe-resizer-super, on the other hand, does not try to call the close method at all when unmounting, resulting in "Iframe not found" warnings.

@davidjbradshaw
Copy link
Owner

davidjbradshaw commented Apr 26, 2018

I'm pretty sure when I tried the one I suggested a year ago it worked. Can you give the whole component ago? It get's the ref in a different way to your code.

@mihkeleidast
Copy link
Author

Unfortunately using that is not an option at all, since it hasn't been updated in a long time and still defines props via React.PropTypes, which does not exist anymore (I'm using a newer version of React).

But I tried changing the way i get the reference, that did not work as well.

@mihkeleidast
Copy link
Author

Created a sandbox to demo the problem: https://codesandbox.io/s/23lxljzwoy

@davidjbradshaw
Copy link
Owner

Just had a look at the sandbox and pretty sure that never used to throw an error. It is clearly testing for the node before deleting it!

In fact MDN even lists this example on how to safely remove a node from the DOM

if (node.parentNode) {
  // remove a node from the tree, unless 
  // it's not in the tree already
  node.parentNode.removeChild(node);
}

I'm struggling to see how that differs from the line you highlighted.

if (iframe.parentNode) { iframe.parentNode.removeChild(iframe); }

I'd rather fix this properly, than add a flag as that risks creating a memory leak. However, currently I don't see how a node can not be a child of it's own parent!

Welcome any ideas and insights you might have.

@davidjbradshaw
Copy link
Owner

davidjbradshaw commented Apr 29, 2018

A bit more digging and I came across this.

https://stackoverflow.com/questions/11600301/removing-an-element-through-the-parentnode-removechild-throws-a-dom-exception-8

It seems that their is a race condition in the browser, as the DOM is updated whilst we check it in the if statement. I've added a try/catch around this, as that sadly seems to be the only completely safe way to deal with this.

Update released as v3.6.1

@mihkeleidast
Copy link
Author

@davidjbradshaw problem still exists with 3.6.1: https://codesandbox.io/s/l45zxp35rm

in the repo i don't see any changes to source files. in the build folder the unminified files haven't been updated, too, and node points to the unminified files when importing.

@mihkeleidast
Copy link
Author

@davidjbradshaw any updates on this?

@davidjbradshaw
Copy link
Owner

I've just written my own React component for iframe-resizer

https://github.com/davidjbradshaw/iframe-resizer-react

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

No branches or pull requests

2 participants