Skip to content
This repository has been archived by the owner on Jul 29, 2019. It is now read-only.

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

Open
maraisr opened this issue Dec 29, 2017 · 7 comments
Open

Comments

@maraisr
Copy link

maraisr commented Dec 29, 2017

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
Copy link
Author

maraisr commented Jan 4, 2018

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
Copy link

tbaltrushaitis commented Jan 4, 2018

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
Copy link
Author

maraisr commented Jan 4, 2018

@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
Copy link

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

@maraisr
Copy link
Author

maraisr commented Jan 4, 2018

Its close enough @tbaltrushaitis

@maraisr
Copy link
Author

maraisr commented Feb 12, 2018

@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
Copy link
Author

maraisr commented Apr 12, 2018

@yotamberk bump

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants