Add nodes to DocumentFragments before attaching #6251
Conversation
|
Can we do .nodeType instead? That's how we check types everywhere else. |
|
Remove debugger too. ;) |
|
Confirmed the test case does work now in IE11. Should we only do this if |
|
Doesn't matter much since insertTreeChildren is a noop if enableLazy is false. |
|
👍 I also tested Edge which seemed to be hitting this too (test case: http://playground.zpao.com/tmp/react_6246/) |
Add nodes to DocumentFragments before attaching
|
@zpao updated the pull request. |
| @@ -53,8 +53,16 @@ function insertTreeChildren(tree) { | |||
|
|
|||
| var insertTreeBefore = createMicrosoftUnsafeLocalFunction( | |||
| function(parentNode, tree, referenceNode) { | |||
| parentNode.insertBefore(tree.node, referenceNode); | |||
| insertTreeChildren(tree); | |||
| // Document Fragments in IE11, Edge (and possibly others) won't update | |||
There was a problem hiding this comment.
Sorry if it was unclear: this is how doc fragments work in all browsers. But we insert things in a different order in IE11 and Edge which is why they exhibit this bug.
There was a problem hiding this comment.
Would running a separate test with a mocked "is IE" flag help prevent the issue?
There was a problem hiding this comment.
Yeah, we should run jest tests with enableLazy on. :\
This is based on a quick chat with @spicyj. I haven't tested in IE11 yet (setting that up now), but after ensuring Firefox is running in lazy mode here, this does do what Ben was thinking, at least for the insert case. I need to do some more testing and see if this is an issue with updates (I think we should be ok there but will check).
Fixes #6246.