Validation fails when running via JsDom in NodeJs

[bartbutenaers]

Dear,

we are integrating the powerful Plotly library into the open-source Node-RED iot framework, which is running on NodeJs.

Running Plotly on the NodeJs server (via JsDom) works fine, but calling the validate function fails.
A small program to reproduce the problem (remark: the jsdom and plotly.js-dist NPM packages need to be installed):

const jsdom = require('jsdom');
const vm = require('vm');  
const fs = require('fs');

var plotlyServerDom = new jsdom.JSDOM('', { runScripts: 'dangerously'});
 
// Mock a few things that JSDOM doesn't support out-of-the-box
plotlyServerDom.window.HTMLCanvasElement.prototype.getContext = function() { return null; };
plotlyServerDom.window.URL.createObjectURL = function() { return null; };

// Run Plotly inside Jsdom
var plotlyJsPath = require.resolve("plotly.js-dist");
var plotlyJsSource = fs.readFileSync(plotlyJsPath, 'utf-8');
plotlyServerDom.window.eval(plotlyJsSource);

var data = [];
var trace ={};
trace.name = "mytrace"; 
trace.type = "scatter";
trace.x = ["2020-09-06 09:10:49"];
trace.y = [5];
trace.opacity = 1;
data.push(trace);

var layout = {};
layout.title = {};
layout.title.text = "mytitle";

var result = plotlyServerDom.window.Plotly.validate(data, layout);

if (result) {
    console.log(result);
}
else {
    console.log("Validation ok");
}

The result is:

[
{
code: 'object',
container: 'layout',
trace: null,
path: '',
astr: '',
msg: 'The layout argument must be linked to an object container'
},
{
code: 'object',
container: 'data',
trace: 0,
path: '',
astr: '',
msg: 'Trace 0 in the data argument must be linked to an object container'
}
]

Which means that Plotly doesn't recognize my input parameters as Javascript objects, which is checked in the isPlainObject function:

The second check fails, so the input is not considered as a Javascript object...
Seems that the problem is caused by this:

1. JsDom runs its own vm (virtual machine) in NodeJs.
2. That vm gets its very own instance of Object.
3. The prototype of an object created inside the vm will not be the exact equal to the prototype of the Object outside the vm (i.e. in my program).
4. So the equality in the IF condition will fail ...

You can find a prove of this theory in the next program. Here a global function createObject is declared inside the vm, and called from outside the vm. This allows us to create an empty Javascript object inside the vm, return it back to us so we can add data to the object, and then we pass the object to Plotly (which is running in the Jsdom vm):

const jsdom      = require('jsdom');
const vm         = require('vm');  
const fs         = require('fs');

var plotlyServerDom = new jsdom.JSDOM('', { runScripts: 'dangerously'});
 
// Mock a few things that JSDOM doesn't support out-of-the-box
plotlyServerDom.window.HTMLCanvasElement.prototype.getContext = function() { return null; };
plotlyServerDom.window.URL.createObjectURL = function() { return null; };
    
// Script to add global functions (to create a Javascript object) in the server DOM context
const script = new vm.Script(`
    function createObject() {
        return {};
    }
`);

// Execute the script in the VM (of jsDom), so that the global function is added to the VM context
const serverDomVmContext = plotlyServerDom.getInternalVMContext();
script.runInContext(serverDomVmContext);
    
var plotlyJsPath = require.resolve("plotly.js-dist");
var plotlyJsSource = fs.readFileSync(plotlyJsPath, 'utf-8');
plotlyServerDom.window.eval(plotlyJsSource);

var data = [];
var trace = plotlyServerDom.window.createObject();
trace.name = "mytrace"; 
trace.type = "scatter";
trace.x = ["2020-09-06 09:10:49"];
trace.y = [5];
trace.opacity = 1;
data.push(trace);

var layout = plotlyServerDom.window.createObject();
layout.title = plotlyServerDom.window.createObject();
layout.title.text = "mytitle";

var result = plotlyServerDom.window.Plotly.validate(data, layout);

if (result) {
    console.log(result);
}
else {
    console.log("Validation ok");
}

And that works fine, and the validation is succesful.

We could use this latter program as a workaround, but it becomes harder when the number of nested levels increases. Because all objects that we receive from anywhere in the NodeJs application, we need to create an new object for and copy all the properties. Which is of course not a very neat solution ...

So we would like to ask if it is possible to remove the second condition from the IF statement, since we think that the first condition is enough:

Thanks for your time!
Bart Butenaers

[archmoj]

@bartbutenaers
Thanks for writing in.
Hmm... instead of removing this condition, maybe we could relax it only when running with node.js or jsdom.
Curious to know if this commit 783de52 can help?
Here you could find a build in case you wanted to test
https://87740-45646037-gh.circle-artifacts.com/0/dist/plotly.min.js

[alexcjohnson]

@archmoj I'm a little wary of that test for node, as anyone can set their window name to whatever they want. And anyway it's not node per se, but the jsDom VM that's causing issues. But anyway if we do find a reasonable test we can put it up above in the special imagetest block:

plotly.js/src/lib/is_plain_object.js

Lines 19 to 22 in 783de52

	// N.B. isPlainObject(new Constructor()) will return true in `imagetest`
	if(window && window.process && window.process.versions) {
	return Object.prototype.toString.call(obj) === '[object Object]';
	}

That comment alludes to the reason for the second part of the test: that we don't want to accept more complex objects

function f() {}
var a = new f();
Object.prototype.toString.call(a)
> "[object Object]"

This isn't a real huge worry, since the majority of our users are coming in via JSON one way or another (plotly.py, dash, etc) but I still don't think we want to completely drop that condition.

@bartbutenaers TBH I don't know anymore what the condition if(window && window.process && window.process.versions) is supposed to accomplish, but if we can't find a simple solution to this you could hijack that:

plotlyServerDom.window.process = {versions: 1};

Alternatively you might be able to go through JSON as well:

var data2 = plotlyServerDom.window.JSON.parse(JSON.stringify(data));
var layout2 = plotlyServerDom.window.JSON.parse(JSON.stringify(layout));

[bartbutenaers]

Hello @archmoj and @alexcjohnson,

Thanks for the fast and kind feedback!
I don't want to mess up any other projects that are already using Plotly, by changing the IF conditions.
So I have tested your workaround:

plotlyServerDom.window.process = {versions: 1};

And that works like a charm. Much cleaner than my own workaround.
It is also good from a performance point of view, since I don't have to copy anymore data from one object to another.
How stupid that I haven't thought about this myself...

So this workaround is more than enough for my use case!
Unless you guys still want to update the IF condition for some reason, you can close this issue.

Thanks a lot for your time!!
Bart

[alexcjohnson]

Great, I'm glad that worked for you @bartbutenaers 🎉 But let's leave this issue open until we either (1) find a more robust way to test for objects so this isn't needed, (2) create and document a more pleasant way to use the looser test, or (3) add this opaque workaround to the official docs.

[alexcjohnson]

I'm starting to think it would be OK to just drop the second part of this test - again, the only concern is if someone passes in an object created with new f() or new MyClass(), which isn't possible for anyone coming in through JSON. It would be possible in Dash using a clientside callback, but you'd really have to work hard to do that! So for the most part this affects JS-only users - whether using plotly.js directly or via some JS framework. And at that point we're trading off a very hypothetical case where we treat an invalid input as valid for several known problems that require tedious debugging and a hacky workaround.

I did a little digging, and I did find one test that pretty reliably distinguishes a plain object from a more complex constructed object: look for a property that is usually inherited from Object, for example:

Object.getPrototypeOf(obj).hasOwnProperty('hasOwnProperty')

Which you can trick by overriding hasOwnProperty in your class, but perhaps at that point you're on your own 😏

Anyway either removing the second condition or replacing it with something like the test above could be the way to go.