[Timeline] item.parent === null - "Cannot read property 'top' of null"

maraisr commented 6 years ago

Component: Timeline Version: 4.21.0

The issue doesn't seem to happen often, maybe 1 in 4. But in the below code snippet, the line in red seems to throw due to group being null. Is there a race condition somewhere I should be aware about? Am I best off not initializing the timeline until I have events ready to go?

Thinking, is the setItems method thread safe, as in, if I call setItems now, then 50ms later call it again with a completely different set of items, is that maybe why things break?

Example:

const timeline = new VisTimeline(
    document.getElementById('my-timeline'),
    [],
    {
        align: 'center',
        maxHeight: '300px',
        height: '300px',
        autoResize: true,
        stackSubgroups: true,
        zoomable: true,
        showMajorLabels: true,
        showMinorLabels: true,
    }
);

getMyDataFromAHTTPCall()
    .then(arrayOfCompaitbleEvents => {
        timeline.setItems(new DataSet(arrayOfCompaitbleEvents));
    });

Stack:

Cannot read property 'top' of null
TypeError: Cannot read property 'top' of null
    at getItemVerticalScroll (node_modules/vis/dist/vis.js:41146:22)
    at Timeout.setFinalVerticalPosition [as _onTimeout] (node_modules/vis/dist/vis.js:41057:33)

Source:

function getItemVerticalScroll(timeline, item) {
  var leftHeight = timeline.props.leftContainer.height;
  var contentHeight = timeline.props.left.height;

  var group = item.parent;
- var offset = group.top;
  var shouldScroll = true;
  var orientation = timeline.timeAxis.options.orientation.axis;

dist/vis.js:41146:22

maraisr commented 6 years ago

To reproduce:

// -- DOM in Node --
const { JSDOM } = require('jsdom');

const jsDomInstance = new JSDOM(
    '<html><head><script></script></head><body></body></html>',
    {
        pretendToBeVisual: true
    }
);

global['window'] = jsDomInstance.window;
global['document'] = global['window'].document;

global['Element'] = global['window'].Element;
global['requestAnimationFrame'] = global['window'].requestAnimationFrame;

// -- Script --

const { Timeline, DataSet } = require('vis');
const uuid = require('uuid').v4;

const div = document.createElement('div');

const timeline = new Timeline(div, []);

const events = makeEvents(3);

timeline.setItems(new DataSet(events));

setTimeout(() => {
    timeline.setItems(new DataSet(makeEvents(4)));
}, 1);

timeline.focus([events[0].id], { animation: false });

function createEvent(dateInput) {
    return {
        start: (date => (date.setDate(dateInput), date))(new Date()),
        id: uuid(),
        content: 'content'
    };
}

function makeEvents(count) {
    return new Array(count)
        .fill(0)
        .map((val, index) => createEvent(index + 1));
}

tbaltrushaitis commented 6 years ago

comment #355170254

To reproduce:

// -- DOM in Node -- const { JSDOM } = require('jsdom');

hey guys, come on, isn't is an almende vis library is frontend library for visualization of data but NOT FOR Node.js? Feel free to correct me if I'm wrong.

Thank you!

maraisr commented 6 years ago

@tbaltrushaitis and yet (https://github.com/almende/vis/blob/master/test/TimelineItemSet.test.js#L6) their own unit tests uses Node.

My issue originates out of Mocha unit tests. The issue is a valid issue, resolved in my PR. It came from items in a dataSet not existing in future event loops.

tbaltrushaitis commented 6 years ago

Browser's window is NOT the same as object produced by JSDOM.

maraisr commented 6 years ago

Its close enough @tbaltrushaitis

maraisr commented 6 years ago

@yotamberk do you think you can release this sometime soon as a patch - our company is eagerly waiting on it. If not that is cool, we can fork.

maraisr commented 6 years ago

@yotamberk bump

almende / vis

[Timeline] item.parent === null - "Cannot read property 'top' of null" #3753