Skip to content

Test plotly.min.js in Require.js environment #914

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 7, 2016
Merged
Show file tree
Hide file tree
Changes from all 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
106 changes: 53 additions & 53 deletions dist/plotly.min.js

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@
"open": "0.0.5",
"prepend-file": "^1.3.0",
"prettysize": "0.0.3",
"requirejs": "^2.3.1",
"through2": "^2.0.0",
"uglify-js": "^2.6.1",
"watchify": "^3.7.0",
Expand Down
2 changes: 2 additions & 0 deletions tasks/bundle.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ var UglifyJS = require('uglify-js');
var constants = require('./util/constants');
var common = require('./util/common');
var compressAttributes = require('./util/compress_attributes');
var patchMinified = require('./util/patch_minified');
var doesFileExist = common.doesFileExist;

/*
Expand Down Expand Up @@ -83,6 +84,7 @@ function _bundle(pathToIndex, pathToBundle, opts) {

if(outputMinified) {
var minifiedCode = UglifyJS.minify(buf.toString(), constants.uglifyOptions).code;
minifiedCode = patchMinified(minifiedCode);

fs.writeFile(pathToMinBundle, minifiedCode, function(err) {
if(err) throw err;
Expand Down
11 changes: 11 additions & 0 deletions tasks/pretest.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ var common = require('./util/common');
makeCredentialsFile();
makeSetPlotConfigFile();
makeTestImageFolders();
makeRequireJSFixture();

// Create a credentials json file,
// to be required in jasmine test suites and test dashboard
Expand Down Expand Up @@ -52,6 +53,16 @@ function makeTestImageFolders() {
makeOne(constants.pathToTestImagesDiff, 'test image diff folder');
}

// Make script file that define plotly in a RequireJS context
function makeRequireJSFixture() {
var bundle = fs.readFileSync(constants.pathToPlotlyDistMin, 'utf-8'),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Important I chose to test Require.js import with the dist/plotly.min.js bundle only.

That means that Require.js will only be tested on npm version commits (when the dist/ is updated).

Main reason for this shotcut: minifying the full plotly.js bundle take about 1 minute on CI.

template = 'define(\'plotly\', function(require, exports, module) { {{bundle}} });',
index = template.replace('{{bundle}}', bundle);

common.writeFile(constants.pathToRequireJSFixture, index);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this basically mimics how plotly.py inject plotly.min.js - see here.

logger('make build/requirejs_fixture.js');
}

function logger(task) {
console.log('ok ' + task);
}
2 changes: 2 additions & 0 deletions tasks/util/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ module.exports = {

pathToJasmineTests: path.join(pathToRoot, 'test/jasmine/tests'),
pathToJasmineBundleTests: path.join(pathToRoot, 'test/jasmine/bundle_tests'),
pathToRequireJS: path.join(pathToRoot, 'node_modules', 'requirejs', 'require.js'),
pathToRequireJSFixture: path.join(pathToBuild, 'requirejs_fixture.js'),

// this mapbox access token is 'public', no need to hide it
// more info: https://www.mapbox.com/help/define-access-token/
Expand Down
15 changes: 15 additions & 0 deletions tasks/util/patch_minified.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
var STR_TO_REPLACE = 'require("+a(r)+");';
var STR_NEW = 'require("+ a(r) +");';

/* Uber hacky in-house fix to
*
* https://github.com/substack/webworkify/issues/29
*
* so that plotly.min.js loads in Jupyter NBs, more info here:
*
* https://github.com/plotly/plotly.py/pull/545
*
*/
module.exports = function patchMinified(minifiedCode) {
return minifiedCode.replace(STR_TO_REPLACE, STR_NEW);
};
16 changes: 16 additions & 0 deletions test/jasmine/bundle_tests/requirejs_test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
describe('plotly.js + require.js', function() {
'use strict';

it('should preserve require.js globals', function() {
expect(window.requirejs).toBeDefined();
expect(window.define).toBeDefined();
expect(window.require).toBeDefined();
});

it('should be able to import plotly.min.js', function(done) {
require(['plotly'], function(Plotly) {
expect(Plotly).toBeDefined();
done();
});
});
});
10 changes: 10 additions & 0 deletions test/jasmine/karma.conf.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,13 @@
*
*/

var constants = require('../../tasks/util/constants');

var arg = process.argv[4];

var testFileGlob = arg ? arg : 'tests/*_test.js';
var isSingleSuiteRun = (arg && arg.indexOf('bundle_tests/') === -1);
var isRequireJSTest = (arg && arg.indexOf('bundle_tests/requirejs') !== -1);

var pathToMain = '../../lib/index.js';
var pathToJQuery = 'assets/jquery-1.8.3.min.js';
Expand Down Expand Up @@ -113,6 +116,13 @@ if(isSingleSuiteRun) {
func.defaultConfig.preprocessors[pathToMain] = ['browserify'];
func.defaultConfig.preprocessors[testFileGlob] = ['browserify'];
}
else if(isRequireJSTest) {
func.defaultConfig.files = [
constants.pathToRequireJS,
constants.pathToRequireJSFixture,
testFileGlob
];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: don't browserify the bundle_tests/requirejs_test.js suite as browserify and require.js don't get along.

}
else {
func.defaultConfig.files = [
pathToJQuery,
Expand Down