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

riot+compiler.js:1245 Uncaught NotFoundError: Failed to execute 'insertBefore' on 'Node': The node before which the new node is to be inserted is not a child of this node - when adding to an 'each' structure from a recursively created element onlick #1962

Closed
TheNavigateur opened this Issue Aug 29, 2016 · 8 comments

Comments

Projects
None yet
5 participants
@TheNavigateur

TheNavigateur commented Aug 29, 2016

Help us to manage our issues by answering the following:

  1. Describe your issue:

In riot v2.6.1 I get:

riot+compiler.js:1245 Uncaught NotFoundError: Failed to execute 'insertBefore' on 'Node': The node before which the new node is to be inserted is not a child of this node

when I try to add the first element to an 'each' structure on an 'onclick' of an element that was generated recursively.

When I change the following line:

else root.insertBefore(tag.root, tags[i].root) // #1374 some browsers reset selected here

to this:

else tag.root.parentNode.insertBefore(tags[i].root, tag.root)

The problem seems to go away, although I don't know whether or not this is a solution.

  1. Can you reproduce the issue?

Unfortunately, when I try to reproduce the recursive tag structure like I have in my app, using the jsfiddle template, I get a stack overflow, even though this doesn't happen in my app when I am using tag files etc.

Here is what I have so far:

http://jsfiddle.net/26dxw9ox/3/ (Just a stack overflow crash upon start, so doesn't reproduce the issue yet)

  1. On which browser/OS does the issue appear?

Windows 10, Chrome

  1. Which version of Riot does it affect?

2.6.1

  1. How would you tag this issue?
    • Question
    • Bug
    • Discussion
    • Feature request
    • Tip
    • Enhancement
    • Performance
@pavel

This comment has been minimized.

Show comment
Hide comment
@pavel

pavel Aug 29, 2016

Related to #1649.
Probably fix was not complete, as it was applied only to this lines while this lines were not changed.

pavel commented Aug 29, 2016

Related to #1649.
Probably fix was not complete, as it was applied only to this lines while this lines were not changed.

@GianlucaGuarini

This comment has been minimized.

Show comment
Hide comment
@GianlucaGuarini

GianlucaGuarini Aug 29, 2016

Member

This issue was fixed in riot@next http://jsfiddle.net/gianlucaguarini/y10L7617/ and it's a duplicate of #1020

Member

GianlucaGuarini commented Aug 29, 2016

This issue was fixed in riot@next http://jsfiddle.net/gianlucaguarini/y10L7617/ and it's a duplicate of #1020

@GianlucaGuarini

This comment has been minimized.

Show comment
Hide comment
@GianlucaGuarini

GianlucaGuarini Aug 29, 2016

Member

@TheNavigateur please avoid recursive tags using riot@2 You can append create recursively new tags in runtime whenever you need it but avoid to place them already in the templates

Member

GianlucaGuarini commented Aug 29, 2016

@TheNavigateur please avoid recursive tags using riot@2 You can append create recursively new tags in runtime whenever you need it but avoid to place them already in the templates

@pavel

This comment has been minimized.

Show comment
Hide comment
@pavel

pavel Aug 29, 2016

The issue is still there in riot@next.

To reproduce:
Go to: http://jsfiddle.net/k95vL8te/2/ and open dev console.

  1. Click "Folder2" to see files in "Folder2".
  2. Click "Remove" on any file listed
  3. Click "Folder1" to see files in "Folder1".

Problem:

NotFoundError: Failed to execute 'insertBefore' on 'Node': The node before which the new node is to be inserted is not a child of this node.

pavel commented Aug 29, 2016

The issue is still there in riot@next.

To reproduce:
Go to: http://jsfiddle.net/k95vL8te/2/ and open dev console.

  1. Click "Folder2" to see files in "Folder2".
  2. Click "Remove" on any file listed
  3. Click "Folder1" to see files in "Folder1".

Problem:

NotFoundError: Failed to execute 'insertBefore' on 'Node': The node before which the new node is to be inserted is not a child of this node.

@GianlucaGuarini

This comment has been minimized.

Show comment
Hide comment
@GianlucaGuarini

GianlucaGuarini Aug 29, 2016

Member

@pavel you are right this issue still exists in our latest alpha release http://jsfiddle.net/gianlucaguarini/wwc2qakx/ and it must be fixed. Thanks for your demo!

Member

GianlucaGuarini commented Aug 29, 2016

@pavel you are right this issue still exists in our latest alpha release http://jsfiddle.net/gianlucaguarini/wwc2qakx/ and it must be fixed. Thanks for your demo!

@pavel

This comment has been minimized.

Show comment
Hide comment
@pavel

pavel Aug 29, 2016

You're welcome. Keep up the good work!

pavel commented Aug 29, 2016

You're welcome. Keep up the good work!

@BuGlessRB

This comment has been minimized.

Show comment
Hide comment
@BuGlessRB

BuGlessRB Aug 31, 2016

My fix looked like this:

--- a/lib/browser/tag/each.js
+++ b/lib/browser/tag/each.js
@@ -153,7 +153,7 @@ export default function _each(dom, parent, expr) {
       var
         _mustReorder = mustReorder && typeof item == T_OBJECT &&
          (!hasKeys || expr.inkey),
-        oldPos = oldItems.indexOf(item),
+        oldPos = oldItems.indexOf(item), tagsi = tags[i],
         pos = ~oldPos && _mustReorder ? oldPos : i,
         // does a tag exist in this position?
         tag = tags[pos], domToInsert
@@ -186,8 +186,9 @@ export default function _each(dom, parent, expr) {
         // this tag must be insert
         else {
           if (isVirtual)
-            makeVirtual(tag, root, tags[i])
-          else root.insertBefore(domToInsert, tags[i].root)
+            makeVirtual(tag, root, tagsi)
+          else root.insertBefore(domToInsert,
+             tagsi.root.parentNode && tagsi.root)
           oldItems.splice(i, 0, item)
         }

Not sure if this is optimal, but it works so far.

BuGlessRB commented Aug 31, 2016

My fix looked like this:

--- a/lib/browser/tag/each.js
+++ b/lib/browser/tag/each.js
@@ -153,7 +153,7 @@ export default function _each(dom, parent, expr) {
       var
         _mustReorder = mustReorder && typeof item == T_OBJECT &&
          (!hasKeys || expr.inkey),
-        oldPos = oldItems.indexOf(item),
+        oldPos = oldItems.indexOf(item), tagsi = tags[i],
         pos = ~oldPos && _mustReorder ? oldPos : i,
         // does a tag exist in this position?
         tag = tags[pos], domToInsert
@@ -186,8 +186,9 @@ export default function _each(dom, parent, expr) {
         // this tag must be insert
         else {
           if (isVirtual)
-            makeVirtual(tag, root, tags[i])
-          else root.insertBefore(domToInsert, tags[i].root)
+            makeVirtual(tag, root, tagsi)
+          else root.insertBefore(domToInsert,
+             tagsi.root.parentNode && tagsi.root)
           oldItems.splice(i, 0, item)
         }

Not sure if this is optimal, but it works so far.

@tlghn

This comment has been minimized.

Show comment
Hide comment

tlghn commented Dec 23, 2016

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