TypeError: Cannot read property 'root' of undefined' - _each #1581

Closed
dylan opened this Issue Feb 8, 2016 · 17 comments

Projects

None yet

3 participants

@dylan
dylan commented Feb 8, 2016

I'm having an issue where it seems like on update we're trying to reorder a tag using an index that's outside the bounds of the total number of tags at our disposal. I don't know enough about the internals to recommend a fix or create a 100% reproducible mockup though. :(

It's important to note that both tags.length and i are 148 so tags[i].root fails.

screen shot 2016-02-08 at 1 55 07 pm-1

@dylan
dylan commented Feb 8, 2016

On second thought, you can reproduce it by doing the following:

  • Visit http://burningsail.com
  • Click in the search box and type George
  • Then change search terms by replacing George with L

And the exception should occur. Let me know if you'd like me to put the non-minified version up to make troubleshooting easier...

@GianlucaGuarini
Member

@dylan I can not see any issue on the site. Can you drill down a simpler example for use to analyze please?

@dylan
dylan commented Feb 9, 2016

It's still there but for some reason it sometimes takes a few times to
manifest. I just had it happen by typing, erasing, and typing more search
terms. Eventually the result list will stop updating.

I'll see if I can create a fiddle or something to replicate it.
On Tue, Feb 9, 2016 at 2:45 AM Gianluca Guarini notifications@github.com
wrote:

@dylan https://github.com/dylan I can not see any issue on the site.
Can you drill down a simpler example for use to analyze please?


Reply to this email directly or view it on GitHub
#1581 (comment).

@dylan
dylan commented Feb 9, 2016

When I setup the example using a debounce, and I only get the error on larger results which makes me wonder if it's a race condition. Like the update is not done before the data changes or something?

Also If I just change the results data to a new array and run update() rather than clearing it, pushing new data and relying on riot to run it's data binding like behavior to update, the problem goes away.

@dylan
dylan commented Feb 9, 2016

Never mind, I'm still seeing it when i do something like i <space> then hit backspace and large result is returned.

@GianlucaGuarini
Member

@dylan I think you have a race condition but I need to investigate if.. have you thought using something like bacon.js? it seems perfect for your use case and it should work perfectly together with riot

@dylan
dylan commented Feb 9, 2016

Good idea. I'll give it a shot. :)

@dylan
dylan commented Feb 13, 2016

Just a heads up, I rewrote the example using bacon.js, and I still got the error on large results. However when I add the "no-reorder" attribute the problem is gone.

@kokujin
kokujin commented Feb 18, 2016

I am having this problem too, where can I read up on the "no-reorder" option @dylan ?

@GianlucaGuarini
Member

I need a way to reproduce this issue to fix it. Can you create a test case guys please?

@dylan
dylan commented Feb 23, 2016

I setup a test case to see what the issue may be but I'm not sure.
I have two scenarios:

Scenario 1

http://plnkr.co/edit/DUeDgjRDkIR2ng93wupj
In this plunk, if you type G in the search box, then select it and replace it with the letter L, then go back to G it fails when no-reorder is not on the loop. When no-reorder is on the loop it works great.

Scenario 2

http://plnkr.co/edit/gz2cZ9khusdIbSLE40du?p=preview
In this plunk I setup two arrays, one big, one small. But each item is identical (this may be a part of the issue?). I alternate the sizes to see if it was a rendering speed issue or something, but I don't think that's it.

I could further change it to use different objects at each index (in case you're memoizing?) to try and trigger a sort or something. Also, in my first example riot is given a new array every time it's rendered, so maybe that's it?

@GianlucaGuarini
Member

@dylan thanks I will have a look

@GianlucaGuarini
Member

The issue was fixed but I was unable to reproduce it in a test

@dylan
dylan commented Feb 25, 2016

Excellent, thanks!

@GianlucaGuarini GianlucaGuarini added a commit that closed this issue Feb 27, 2016
@GianlucaGuarini GianlucaGuarini closes #1581 9e75ce2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment