Clear previous children when SVG node doesn't have innerHTML #11108
Conversation
|
Deploy preview ready! Built with commit 3fad3b5 |
|
Have you tested it in IE? |
|
The only IE environment I have is IE11 Windows 10 on Parallels and it's now working there (that's the environment I originally encountered this issue) |
|
@gaearon @OriR I have confirmed this works in IE9 and IE10! I made an SVG test fixture for it here: http://react-svg-innerhtml.surge.sh/svg We can decide if we want to merge this test fixture later. I can send out a PR for that later. |
| return prop === 'innerHTML' ? false : prop in target; | ||
| }, | ||
| get: (target, prop) => { | ||
| if (prop === 'appendChild') { |
There was a problem hiding this comment.
Why do we need this? If you append a child to another parent, it will be removed from the current parent.
Isn't just
get: (target, prop) => {
return target[prop];
},enough?
There was a problem hiding this comment.
In the comment here it says that JSDOM doesn't remove the node from the current parent when moving nodes using appendChild, though I didn't verify this myself.
I could check and see if that's still relevant, if not then I can remove the get handler altogether.
There was a problem hiding this comment.
Please check this, yes. The comments might be very outdated.
There was a problem hiding this comment.
You were right, this was no longer necessary 🎉
Removed the get handler altogether!
|
Thanks. |
Fixes #10013
I also fixed the UT, but they only work in node 6+ (using Proxy).
I saw that node 4-5 are still supported as development environments, so if this upgrade is not possible right now I'll revert them.
Couldn't get the tests to work otherwise 😞