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

Use mock canvas object replacing canvas #3518

Merged
merged 4 commits into from
Oct 13, 2017
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,31 @@ module: {

There is also an [demo-project](https://github.com/mojoaxel/vis-webpack-demo) showing the integration of vis.js using webpack.


## Using module `canvas`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this section should be better a sub-part of "Installation".


The unit tests use a mock object for canvas, which is severely limited in its functionality. It might be preferable
to use module `canvas` supplies a canvas object for usage with `node.js`. This has a functional implementation of the 2D context.

Reasons to use `canvas:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need this "Reasons to use canvas" section at all.


- You want to run the `mocha` tests as complete as possible
- You want to run `Network` or `Graph3D` on `node.js` only, e.g. server-side.

The unit tests will only use a mock object for canvas, if there is no `canvas` element already supplied. So, if the `canvas` module is installed, that will take precedence.


The issue with `canvas` is that it has an external dependency to `cairo`. This needs to be installed outside of the regular install as done by `npm`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


- In order to fully install `canvas` on Windows, follow [these instructions](https://github.com/Automattic/node-canvas/wiki/Installation---Windows).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just link to the canvas-wiki and remove this whole section.

- To install on linux, you will need to do something along the lines of the following:

> sudo apt-get install libcairo2-dev libjpeg8-dev libpango1.0-dev libgif-dev build-essential g++
> sudo npm -g install canvas

- To install on OSX, [These instructions](https://github.com/Automattic/node-canvas/wiki/Installation---OSX) look the most promising.


## Test

To test the library, install the project dependencies once:
Expand Down
12 changes: 6 additions & 6 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,30 +33,30 @@
"watch-dev": "gulp watch --bundle"
},
"dependencies": {
"canvas": "^1.6.7",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Canvas" is not needed als dependency because it is only used during development. Should be removed here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. I must have placed it there again during testing.

"emitter-component": "^1.1.1",
"moment": "^2.18.1",
"propagating-hammerjs": "^1.4.6",
"hammerjs": "^2.0.8",
"keycharm": "^0.2.0"
"keycharm": "^0.2.0",
"moment": "^2.18.1",
"propagating-hammerjs": "^1.4.6"
},
"devDependencies": {
"async": "^2.5.0",
"babel-core": "^6.25.0",
"babel-loader": "^7.1.1",
"babel-polyfill": "^6.23.0",
"babel-plugin-transform-es3-member-expression-literals": "^6.22.0",
"babel-plugin-transform-es3-property-literals": "^6.22.0",
"babel-plugin-transform-runtime": "^6.23.0",
"babel-polyfill": "^6.23.0",
"babel-preset-es2015": "^6.24.1",
"babel-runtime": "^6.23.0",
"babelify": "^7.3.0",
"canvas": "^1.6.5",
"clean-css": "^4.1.7",
"eslint": "^4.3.0",
"gulp": "^3.9.1",
"gulp-eslint": "^4.0.0",
"gulp-clean-css": "^3.7.0",
"gulp-concat": "^2.6.1",
"gulp-eslint": "^4.0.0",
"gulp-rename": "^1.2.2",
"gulp-util": "^3.0.8",
"jsdom": "9.12.0",
Expand Down
2 changes: 2 additions & 0 deletions test/Graph3d.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ var assert = require('assert');
var vis = require('../dist/vis');
var Graph3d = vis.Graph3d;
var jsdom_global = require('jsdom-global');
var canvasMockify = require('./canvas-mock');
var stdout = require('test-console').stdout;
var Validator = require("./../lib/shared/Validator").default;
//var {printStyle} = require('./../lib/shared/Validator');
Expand All @@ -16,6 +17,7 @@ describe('Graph3d', function () {
"<div id='mygraph'></div>",
{ skipWindowCheck: true}
);
canvasMockify(window);
this.container = document.getElementById('mygraph');
});

Expand Down
2 changes: 2 additions & 0 deletions test/Label.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ var Label = require('../lib/network/modules/components/shared/Label').default;
var NodesHandler = require('../lib/network/modules/NodesHandler').default;
var util = require('../lib/util');
var jsdom_global = require('jsdom-global');
var canvasMockify = require('./canvas-mock');
var vis = require('../dist/vis');
var Network = vis.network;

Expand Down Expand Up @@ -324,6 +325,7 @@ describe('Network Label', function() {
"<div id='mynetwork'></div>",
{ skipWindowCheck: true}
);
canvasMockify(window);
this.container = document.getElementById('mynetwork');
});

Expand Down
24 changes: 23 additions & 1 deletion test/Network.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,16 @@ var fs = require('fs');
var assert = require('assert');
var vis = require('../dist/vis');
var Network = vis.network;
var jsdom_global = require('jsdom-global');
var stdout = require('test-console').stdout;
var Validator = require("./../lib/shared/Validator").default;
var jsdom_global = require('jsdom-global');
var canvasMockify = require('./canvas-mock');
var {allOptions, configureOptions} = require('./../lib/network/options.js');
//var {printStyle} = require('./../lib/shared/Validator');




Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra blank lines.

Typically, I use 2 blank lines to separate top level class/function declarations, single blank line to separate import groups. Two blank lines to separate imports from constants. Single blank line between nested functions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have my own guidelines which broadly match yours. But this was indeed a good observation.

/**
* Merge all options of object b into object b
* @param {Object} a
Expand Down Expand Up @@ -222,13 +225,16 @@ function checkFontProperties(fontItem, checkStrict = true) {





describe('Network', function () {

before(function() {
this.jsdom_global = jsdom_global(
"<div id='mynetwork'></div>",
{ skipWindowCheck: true}
);
canvasMockify(window);
this.container = document.getElementById('mynetwork');
});

Expand Down Expand Up @@ -324,6 +330,7 @@ describe('Network', function () {
* The real deterrent is eslint rule 'guard-for-in`.
*/
it('can deal with added fields in Array.prototype', function (done) {
var canvas = window.document.createElement('canvas');
Array.prototype.foo = 1; // Just add anything to the prototype
Object.prototype.bar = 2; // Let's screw up hashes as well

Expand Down Expand Up @@ -353,6 +360,9 @@ describe('Network', function () {


describe('Node', function () {
before(function() {
// if (!this.canvasPresent) this.skip();
});
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These before functions should all be removed, imo

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, they should. Artifacts of a previous attempt at a fix.


it('has known font options', function () {
var network = createNetwork({});
Expand Down Expand Up @@ -403,6 +413,9 @@ describe('Node', function () {


describe('Edge', function () {
before(function() {
// if (!this.canvasPresent) this.skip();
});

it('has known font options', function () {
var network = createNetwork({});
Expand Down Expand Up @@ -614,6 +627,9 @@ describe('Edge', function () {


describe('Clustering', function () {
before(function() {
// if (!this.canvasPresent) this.skip();
});


it('properly handles options allowSingleNodeCluster', function() {
Expand Down Expand Up @@ -1204,6 +1220,9 @@ describe('Clustering', function () {


describe('on node.js', function () {
before(function() {
// if (!this.canvasPresent) this.skip();
});

it('should be running', function () {
assert(this.container !== null, 'Container div not found');
Expand All @@ -1217,6 +1236,9 @@ describe('on node.js', function () {


describe('runs example ', function () {
before(function() {
// if (!this.canvasPresent) this.skip();
});

function loadExample(path, noPhysics) {
include(path, this);
Expand Down
90 changes: 90 additions & 0 deletions test/canvas-mock.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
/**
* Set up mock 2D context, for usage in unit tests.
*
* Adapted from: https://github.com/Cristy94/canvas-mock
*/

var myHackedInCanvas; // Use one global canvas instance for all calls to createElement('canvas');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why global?
If that's a good reason, can we call this globalCanvasMock?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, the comment might be a bit off. It's strictly speaking not a global because it doesn't reside in the global namespace. It's a static within the module. I'll adjust the comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And the name is silly; changing it to canvasMock.



function replaceCanvasContext (el) {
el.getContext = function() {
return {
fillRect: function() {},
clearRect: function(){},
getImageData: function(x, y, w, h) {
return {
data: new Array(w*h*4)
};
},
putImageData: function() {},
createImageData: function(){ return []},
setTransform: function(){},
drawImage: function(){},
save: function(){},
fillText: function(){},
restore: function(){},
beginPath: function(){},
moveTo: function(){},
lineTo: function(){},
closePath: function(){},
stroke: function(){},
translate: function(){},
scale: function(){},
rotate: function(){},
arc: function(){},
fill: function(){},

//
// Following added for vis.js unit tests
//

measureText: function(text) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spacing looks off

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, this code is a module which I took from here and adapted it to own needs when I realized it wasn't completely fit for the purpose.

If think you mean this spacing?

      measureText: function(text) {
        return {
          width:>>  <<12*text.length,
          height: 14
        };
      },
};

Otherwise, I don't know what you mean. I have a habit of aligning related stuff, but indeed it's not needed here with only two lines.


Ah wait, there's tabs there. That's probably what you mean. Ah, right, that's from the original. I'll change it to spaces.

return {
width: 12*text.length,
height: 14
};
},
};
}
};


/**
* Overrides document.createElement(), in order to supply a custom canvas element.
*
* In the canvas element, getContext() is overridden in order to supply a simple
* mock object for the 2D context. For all other elements, the call functions unchanged.
*
* The override is only done if there is no 2D context already present.
* This allows for normal running in a browser, and for node.js the usage of 'canvas'.
*
* @param {object} the current global window object. This can possible come from module 'jsdom',
* when running under node.js.
*/
function mockify(window) {
var d = window.document;
var f = window.document.createElement;

// Check if 2D context already present. That happens either when running in a browser,
// or this is node.js with 'canvas' installed.
var ctx = d.createElement('canvas').getContext('2d');
if (ctx !== null && ctx !== undefined) {
//console.log('2D context is present, no need to override');
return;
}

window.document.createElement = function(param) {
if (param === 'canvas') {
if (myHackedInCanvas === undefined) {
myHackedInCanvas = f.call(d, 'canvas');
replaceCanvasContext(myHackedInCanvas);
}
return myHackedInCanvas;
} else {
return f.call(d, param);
}
};
}

module.exports = mockify;