bvaughn / react-virtualized

React components for efficiently rendering large lists and tabular data
http://bvaughn.github.io/react-virtualized/
MIT License
26.31k stars 3.05k forks source link

Error: Cannot read property 'firstElementChild' of undefined #444

Open andersekdahl opened 7 years ago

andersekdahl commented 7 years ago

Hi! First off, big thanks for this library. It's awesome, and it was very easy for us to integrate with.

Since we've gone live with a site using react-virtualized we sometimes see an error occuring that says Cannot read property 'firstElementChild' of undefined where the call stack is only from react-virtualized. The call stack looks like this:

at firstElementChild line 47, column 0 (webpack:///./~/react-virtualized/dist/commonjs/vendor/detectElementResize.js:47)

        var triggers = element.__resizeTriggers__,
            expand = triggers.firstElementChild,
            contract = triggers.lastElementChild,
            expandChild = expand.firstElementChild; // <-- This line
        contract.scrollLeft = contract.scrollWidth;
        contract.scrollTop = contract.scrollHeight;
        expandChild.style.width = expand.offsetWidth + 1 + 'px';

at resetTriggers line 142, column 0 (webpack:///./~/react-virtualized/dist/commonjs/vendor/detectElementResize.js:142)

          /* Listen for a css animation to detect element display/re-attach */
          animationstartevent && element.__resizeTriggers__.addEventListener(animationstartevent, function (e) {
            if (e.animationName == animationName) resetTriggers(element); <-- This line
          });
        }
        element.__resizeListeners__.push(fn);

I've never seen this error myself, and unfortunately I have no idea how to reproduce it. I'm guessing this error occurs when you scroll and then navigate so that we're removing the virtual scroll elements at the same time that the resetTriggers function is called.

The last error we got about this happened on Android 5.0 with Chrome 34.0.1847.76. I'll update this issue if I see if occuring in other browsers.

Please let me know if there's anything I can do to help out with this issue.

bvaughn commented 7 years ago

I recall responding to this issue, but clearly I didn't. Hmm...sorry! I was traveling. Maybe I tried to respond on my phone and it didn't go through or something.

FWIW, the code you're referring to is a fork of sdecima/javascript-detect-element-resize. I've forked it a few times for performance reasons but the triggers bit you're talking about hasn't really been changed.

I don't think I've ever seen the issue you're reporting. Some info about how to reproduce it would be a great first step. Also happily accept a PR if you dig into it. πŸ˜„

bvaughn commented 7 years ago

Closing this for the time being. Can re-open if it repro steps are provided. πŸ˜„

PhilippSpo commented 7 years ago

@bvaughn we can confirm this issue. It happens in Edge (14.14393) on Windows 10 whenever you access a view that uses AutoSizer. It has also been reported in the original javascript-detect-element-resize repo.

bvaughn commented 7 years ago

Thanks for the additional info, @PhilippSpo. Fixes welcome if you get the time to dig into this πŸ˜„

bvaughn commented 7 years ago

PS sdecima/javascript-detect-element-resize seems pretty well abandoned.

I thought about contributing some of the perf/bug fixes I made to my vendored version back to the main project but they wouldn't get merged.

bvaughn commented 7 years ago

@PhilippSpo I just viewed the demo site on Edge 14.14393 in Windows 10 (using Virtual Box) and was unable to reproduce the bug. That's expected, since I've tested RV components against a few versions of IE before to ensure compatibility (and AutoSizer is used on every page on the demo site).

There seems to be something more going on here.

rhagigi commented 7 years ago

There is an open bug on javascript-detect-element-resize about this. https://github.com/sdecima/javascript-detect-element-resize/issues/42

The issue is that it's adding an animationstart event listener during addResizeListener but not removing it during removeResizeListener. The browser is still calling animationstart even though the resizeTriggers element has technically been removed from the dom. As opposed to keeping the event listener and manually removing it during remove, I just added a check in my fork during resetTriggers to check to see if element.__resizeTriggers__ is even there. (it's false -- which is why false.firstElementChild is null (and then it tries to get firstElementChild of that).

The correct solution is to remove the animationstart event listener when the resize listener is removed (something like the code in the github issue) but just to get around it, I just added a check for if (!element) { return; } at the top of resetTriggers

rouzbeh84 commented 7 years ago

I think this ties back to the final few comments in #150 if I'm not mistaken. Trying to track the trail down to see what a possible fix could be. Unless @jontewks has been cooking something up based on what seem to be fairly similar findings in #150. Maybe this info could help out his efforts as well and help move it forward πŸ‘

If I'm totally off base please disregard my comment and proceed as if I never said anything since this is my first dive into what's going on here and I'm just trying to get familiar.

karatechops commented 6 years ago

@rhagigi have you successfully reproduced this? If so could you please provide steps?