{"action":"created","issue":{"url":"https://api.github.com/repos/substack/js-traverse/issues/44","repository_url":"https://api.github.com/repos/substack/js-traverse","labels_url":"https://api.github.com/repos/substack/js-traverse/issues/44/labels{/name}","comments_url":"https://api.github.com/repos/substack/js-traverse/issues/44/comments","events_url":"https://api.github.com/repos/substack/js-traverse/issues/44/events","html_url":"https://github.com/substack/js-traverse/issues/44","id":55067495,"number":44,"title":"this.remove breaks when using splice on an array","user":{"login":"jbeecham","id":4671010,"avatar_url":"https://avatars.githubusercontent.com/u/4671010?v=3","gravatar_id":"","url":"https://api.github.com/users/jbeecham","html_url":"https://github.com/jbeecham","followers_url":"https://api.github.com/users/jbeecham/followers","following_url":"https://api.github.com/users/jbeecham/following{/other_user}","gists_url":"https://api.github.com/users/jbeecham/gists{/gist_id}","starred_url":"https://api.github.com/users/jbeecham/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jbeecham/subscriptions","organizations_url":"https://api.github.com/users/jbeecham/orgs","repos_url":"https://api.github.com/users/jbeecham/repos","events_url":"https://api.github.com/users/jbeecham/events{/privacy}","received_events_url":"https://api.github.com/users/jbeecham/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":2,"created_at":"2015-01-21T19:46:05Z","updated_at":"2016-11-05T15:13:30Z","closed_at":null,"body":"I believe there is an issue when using the remove method for a node. Installed from npm version 0.6.6\n\nIf the node is part of an array, and you remove that node from the array, this changes the size of the parent array. Then when the traverse.forEach function tries to walk to the next node of that array, the array size has changed and the the index is invalid.\n\nOne possible solution is to index arrays backwards from (length - 1) to 0. That way when you splice out and element, the array size changes but the index will always be valid. \n\na quick example to illustrate the point: (code and output below)\n\nI will attempt to remove all nodes with the key \"tileIdx\". You will see that not all are removed if they are in the same parent array. The array index is out of bounds after removing the first element.\n\nBefore: \n{\"expanded\":\"true\",\"folder\":\"true\",\"key\":\"1\",\"title\":\"Caitlin Havener\",\"children\":[{\"expanded\":\"true\",\"folder\":\"true\",\"key\":\"2\",\"title\":\"Sports\",\"children\":[{\"folder\":\"true\",\"key\":\"3\",\"title\":\"Basketball\",\"children\":[{\"key\":\"_2\",\"tileIdx\":\"54bea3403282a1af40c8ac67\"},{\"key\":\"_3\",\"tileIdx\":\"54bea3403282a1af40c8ac67\"}]},{\"folder\":\"true\",\"key\":\"6\",\"title\":\"Football\"}]}]}\n\nCurrent Node: {\"expanded\":\"true\",\"folder\":\"true\",\"key\":\"1\",\"title\":\"Caitlin Havener\",\"children\":[{\"expanded\":\"true\",\"folder\":\"true\",\"key\":\"2\",\"title\":\"Sports\",\"children\":[{\"folder\":\"true\",\"key\":\"3\",\"title\":\"Basketball\",\"children\":[{\"key\":\"_2\",\"tileIdx\":\"54bea3403282a1af40c8ac67\"},{\"key\":\"_3\",\"tileIdx\":\"54bea3403282a1af40c8ac67\"}]},{\"folder\":\"true\",\"key\":\"6\",\"title\":\"Football\"}]}]}\nCurrent Node: \"true\"\nCurrent Node: \"true\"\nCurrent Node: \"1\"\nCurrent Node: \"Caitlin Havener\"\nCurrent Node: [{\"expanded\":\"true\",\"folder\":\"true\",\"key\":\"2\",\"title\":\"Sports\",\"children\":[{\"folder\":\"true\",\"key\":\"3\",\"title\":\"Basketball\",\"children\":[{\"key\":\"_2\",\"tileIdx\":\"54bea3403282a1af40c8ac67\"},{\"key\":\"_3\",\"tileIdx\":\"54bea3403282a1af40c8ac67\"}]},{\"folder\":\"true\",\"key\":\"6\",\"title\":\"Football\"}]}]\nCurrent Node: {\"expanded\":\"true\",\"folder\":\"true\",\"key\":\"2\",\"title\":\"Sports\",\"children\":[{\"folder\":\"true\",\"key\":\"3\",\"title\":\"Basketball\",\"children\":[{\"key\":\"_2\",\"tileIdx\":\"54bea3403282a1af40c8ac67\"},{\"key\":\"_3\",\"tileIdx\":\"54bea3403282a1af40c8ac67\"}]},{\"folder\":\"true\",\"key\":\"6\",\"title\":\"Football\"}]}\nCurrent Node: \"true\"\nCurrent Node: \"true\"\nCurrent Node: \"2\"\nCurrent Node: \"Sports\"\nCurrent Node: [{\"folder\":\"true\",\"key\":\"3\",\"title\":\"Basketball\",\"children\":[{\"key\":\"_2\",\"tileIdx\":\"54bea3403282a1af40c8ac67\"},{\"key\":\"_3\",\"tileIdx\":\"54bea3403282a1af40c8ac67\"}]},{\"folder\":\"true\",\"key\":\"6\",\"title\":\"Football\"}]\nCurrent Node: {\"folder\":\"true\",\"key\":\"3\",\"title\":\"Basketball\",\"children\":[{\"key\":\"_2\",\"tileIdx\":\"54bea3403282a1af40c8ac67\"},{\"key\":\"_3\",\"tileIdx\":\"54bea3403282a1af40c8ac67\"}]}\nCurrent Node: \"true\"\nCurrent Node: \"3\"\nCurrent Node: \"Basketball\"\nCurrent Node: [{\"key\":\"_2\",\"tileIdx\":\"54bea3403282a1af40c8ac67\"},{\"key\":\"_3\",\"tileIdx\":\"54bea3403282a1af40c8ac67\"}]\nCurrent Node: {\"key\":\"_2\",\"tileIdx\":\"54bea3403282a1af40c8ac67\"} **\\* - removing this element\nCurrent Node: undefined **\\* - here array idx of 1 is invalid because array size is now 1\nCurrent Node: {\"folder\":\"true\",\"key\":\"6\",\"title\":\"Football\"}\nCurrent Node: \"true\"\nCurrent Node: \"6\"\nCurrent Node: \"Football\"\n\nFinal output:\n\n{\"expanded\":\"true\",\"folder\":\"true\",\"key\":\"1\",\"title\":\"Caitlin Havener\",\"children\":[{\"expanded\":\"true\",\"folder\":\"true\",\"key\":\"2\",\"title\":\"Sports\",\"children\":[{\"folder\":\"true\",\"key\":\"3\",\"title\":\"Basketball\",\"children\":[{\"key\":\"_3\",\"tileIdx\":\"54bea3403282a1af40c8ac67\"}]},{\"folder\":\"true\",\"key\":\"6\",\"title\":\"Football\"}]}]}\n\nAs you can see the key : _3 node is still there.\n\nCode:\n\nvar traverse = require('traverse');\n\nvar obj = {     \"expanded\" : \"true\",    \"folder\" : \"true\",  \"key\" : \"1\",    \"title\" : \"Caitlin Havener\",    \"children\" : [  {   \"expanded\" : \"true\",    \"folder\" : \"true\",  \"key\" : \"2\",    \"title\" : \"Sports\",     \"children\" : [  {   \"folder\" : \"true\",  \"key\" : \"3\",    \"title\" : \"Basketball\",     \"children\" : [  {   \"key\" : \"_2\",   \"tileIdx\" : \"54bea3403282a1af40c8ac67\" }, {   \"key\" : \"_3\",   \"tileIdx\" : \"54bea3403282a1af40c8ac67\" } ] },   {   \"folder\" : \"true\",  \"key\" : \"6\",    \"title\" : \"Football\" } ] } ] };\n\nconsole.log(\"Before: \\n\" + JSON.stringify(obj) + \"\\n\\n\");\n\ntraverse(obj).forEach(function (x) {\n    console.log(\"Current Node: \" + JSON.stringify(x));\n\n```\nif (x && x.tileIdx)\n    this.remove(true);\n```\n\n});\n\nconsole.log(\"\\nFinal output:\\n\");\nconsole.log(JSON.stringify(obj));\n"},"comment":{"url":"https://api.github.com/repos/substack/js-traverse/issues/comments/258617878","html_url":"https://github.com/substack/js-traverse/issues/44#issuecomment-258617878","issue_url":"https://api.github.com/repos/substack/js-traverse/issues/44","id":258617878,"user":{"login":"dmorehouse","id":2547473,"avatar_url":"https://avatars.githubusercontent.com/u/2547473?v=3","gravatar_id":"","url":"https://api.github.com/users/dmorehouse","html_url":"https://github.com/dmorehouse","followers_url":"https://api.github.com/users/dmorehouse/followers","following_url":"https://api.github.com/users/dmorehouse/following{/other_user}","gists_url":"https://api.github.com/users/dmorehouse/gists{/gist_id}","starred_url":"https://api.github.com/users/dmorehouse/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dmorehouse/subscriptions","organizations_url":"https://api.github.com/users/dmorehouse/orgs","repos_url":"https://api.github.com/users/dmorehouse/repos","events_url":"https://api.github.com/users/dmorehouse/events{/privacy}","received_events_url":"https://api.github.com/users/dmorehouse/received_events","type":"User","site_admin":false},"created_at":"2016-11-05T15:13:30Z","updated_at":"2016-11-05T15:13:30Z","body":"I was able to work around this by using a before callback to reverse the order that arrays are traversed.  \r\n\r\n*NOTE: This only works for forEach traversal and NOT map*\r\n\r\n```\r\nvar traverse = require('traverse');\r\n\r\nvar obj = {\r\n    tree: [\r\n        { content: \"First\" },\r\n        { content: \"abc\" },\r\n        { content: \"Second\" }\r\n    ]\r\n};\r\n\r\nvar result = traverse(obj).forEach(function() {\r\n    if (Array.isArray(this.node)) {\r\n        this.before(function() {\r\n            this.keys = this.keys.reverse();\r\n        });\r\n    }\r\n    if (this.node && this.node.content === 'abc') {\r\n        this.remove();\r\n    }\r\n});\r\n\r\nconsole.dir(result.tree);\r\n```"}}
{"actor":{"display_login":"dmorehouse"}}