Skip to content

Commit ab8b922

Browse files
etpinardetpinard
authored
Merge pull request #2792 from plotly/better-mapbox-minification-fix
Better mapbox minification fix
2 parents 9a92782 + 810e91f commit ab8b922

File tree

8 files changed

+160
-77
lines changed

8 files changed

+160
-77
lines changed

Diff for: ‎.circleci/test.sh

+1-1
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,13 @@ case $1 in
2929

3030
jasmine)
3131
npm run test-jasmine -- --skip-tags=gl,noCI,flaky || EXIT_STATE=$?
32+
npm run test-bundle || EXIT_STATE=$?
3233
exit $EXIT_STATE
3334
;;
3435

3536
jasmine2)
3637
retry npm run test-jasmine -- --tags=gl --skip-tags=noCI,flaky
3738
retry npm run test-jasmine -- --tags=flaky --skip-tags=noCI
38-
npm run test-bundle || EXIT_STATE=$?
3939
exit $EXIT_STATE
4040
;;
4141

Diff for: ‎tasks/test_bundle.js

+37-7
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,46 @@
11
var path = require('path');
2+
var exec = require('child_process').exec;
23
var glob = require('glob');
4+
var runSeries = require('run-series');
35

46
var constants = require('./util/constants');
5-
var common = require('./util/common');
6-
var pathToJasmineBundleTests = path.join(constants.pathToJasmineBundleTests);
7-
7+
var pathToJasmineBundleTests = constants.pathToJasmineBundleTests;
88

9+
/**
10+
* Run all jasmine 'bundle' test in series
11+
*
12+
* To run specific bundle tests, use
13+
*
14+
* $ npm run test-jasmine -- --bundleTest=<name-of-suite>
15+
*/
916
glob(pathToJasmineBundleTests + '/*.js', function(err, files) {
10-
files.forEach(function(file) {
11-
var baseName = path.basename(file);
12-
var cmd = 'npm run test-jasmine -- --bundleTest=' + baseName;
17+
var tasks = files.map(function(file) {
18+
return function(cb) {
19+
var cmd = [
20+
'karma', 'start',
21+
path.join(constants.pathToRoot, 'test', 'jasmine', 'karma.conf.js'),
22+
'--bundleTest=' + path.basename(file),
23+
'--nowatch'
24+
].join(' ');
25+
26+
console.log('Running: ' + cmd);
27+
28+
exec(cmd, function(err) {
29+
cb(null, err);
30+
}).stdout.pipe(process.stdout);
31+
};
32+
});
33+
34+
runSeries(tasks, function(err, results) {
35+
if(err) throw err;
36+
37+
var failed = results.filter(function(r) { return r; });
1338

14-
common.execCmd(cmd);
39+
if(failed.length) {
40+
console.log('\ntest-bundle summary:');
41+
failed.forEach(function(r) { console.warn('- ' + r.cmd + ' failed'); });
42+
console.log('');
43+
process.exit(1);
44+
}
1545
});
1646
});

Diff for: ‎tasks/util/browserify_wrapper.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ module.exports = function _bundle(pathToIndex, pathToBundle, opts, cb) {
4545
}
4646

4747
var b = browserify(pathToIndex, browserifyOpts);
48-
var pending = opts.pathToMinBundle ? 2 : 1;
48+
var pending = pathToMinBundle ? 2 : 1;
4949

5050
function done() {
5151
if(cb && --pending === 0) cb(null);
@@ -58,7 +58,7 @@ module.exports = function _bundle(pathToIndex, pathToBundle, opts, cb) {
5858
}
5959
});
6060

61-
if(opts.pathToMinBundle) {
61+
if(pathToMinBundle) {
6262
bundleStream
6363
.pipe(minify(constants.uglifyOptions))
6464
.pipe(fs.createWriteStream(pathToMinBundle))

Diff for: ‎tasks/util/constants.js

+10-3
Original file line numberDiff line numberDiff line change
@@ -85,10 +85,17 @@ module.exports = {
8585
testContainerHome: '/var/www/streambed/image_server/plotly.js',
8686

8787
uglifyOptions: {
88+
ecma: 5,
8889
mangle: true,
89-
// the compress flag break mapbox-gl,
90-
// TODO find a way to only skip compression on mapbox-gl files
91-
compress: false,
90+
compress: {
91+
// see full list of compress option
92+
// https://github.com/fabiosantoscode/terser#compress-options
93+
//
94+
// need to turn off 'typeofs' to make mapbox-gl work in
95+
// minified bundles, for more info see:
96+
// https://github.com/plotly/plotly.js/issues/2787
97+
typeofs: false
98+
},
9299
output: {
93100
beautify: false,
94101
ascii_only: true

Diff for: ‎test/jasmine/assets/mock_lists.js

+69
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
// list of mocks that should include *all* plotly.js trace modules
2+
3+
var svgMockList = [
4+
['1', require('@mocks/1.json')],
5+
['4', require('@mocks/4.json')],
6+
['5', require('@mocks/5.json')],
7+
['10', require('@mocks/10.json')],
8+
['11', require('@mocks/11.json')],
9+
['17', require('@mocks/17.json')],
10+
['21', require('@mocks/21.json')],
11+
['22', require('@mocks/22.json')],
12+
['airfoil', require('@mocks/airfoil.json')],
13+
['annotations-autorange', require('@mocks/annotations-autorange.json')],
14+
['axes_enumerated_ticks', require('@mocks/axes_enumerated_ticks.json')],
15+
['axes_visible-false', require('@mocks/axes_visible-false.json')],
16+
['bar_and_histogram', require('@mocks/bar_and_histogram.json')],
17+
['basic_error_bar', require('@mocks/basic_error_bar.json')],
18+
['binding', require('@mocks/binding.json')],
19+
['cheater_smooth', require('@mocks/cheater_smooth.json')],
20+
['finance_style', require('@mocks/finance_style.json')],
21+
['geo_first', require('@mocks/geo_first.json')],
22+
['layout_image', require('@mocks/layout_image.json')],
23+
['layout-colorway', require('@mocks/layout-colorway.json')],
24+
['polar_categories', require('@mocks/polar_categories.json')],
25+
['polar_direction', require('@mocks/polar_direction.json')],
26+
['range_selector_style', require('@mocks/range_selector_style.json')],
27+
['range_slider_multiple', require('@mocks/range_slider_multiple.json')],
28+
['sankey_energy', require('@mocks/sankey_energy.json')],
29+
['scattercarpet', require('@mocks/scattercarpet.json')],
30+
['shapes', require('@mocks/shapes.json')],
31+
['splom_iris', require('@mocks/splom_iris.json')],
32+
['table_wrapped_birds', require('@mocks/table_wrapped_birds.json')],
33+
['ternary_fill', require('@mocks/ternary_fill.json')],
34+
['text_chart_arrays', require('@mocks/text_chart_arrays.json')],
35+
['transforms', require('@mocks/transforms.json')],
36+
['updatemenus', require('@mocks/updatemenus.json')],
37+
['violin_side-by-side', require('@mocks/violin_side-by-side.json')],
38+
['world-cals', require('@mocks/world-cals.json')],
39+
['typed arrays', {
40+
data: [{
41+
x: new Float32Array([1, 2, 3]),
42+
y: new Float32Array([1, 2, 1])
43+
}]
44+
}]
45+
];
46+
47+
var glMockList = [
48+
['gl2d_heatmapgl', require('@mocks/gl2d_heatmapgl.json')],
49+
['gl2d_line_dash', require('@mocks/gl2d_line_dash.json')],
50+
['gl2d_parcoords_2', require('@mocks/gl2d_parcoords_2.json')],
51+
['gl2d_pointcloud-basic', require('@mocks/gl2d_pointcloud-basic.json')],
52+
['gl3d_annotations', require('@mocks/gl3d_annotations.json')],
53+
['gl3d_set-ranges', require('@mocks/gl3d_set-ranges.json')],
54+
['gl3d_world-cals', require('@mocks/gl3d_world-cals.json')],
55+
['gl3d_cone-autorange', require('@mocks/gl3d_cone-autorange.json')],
56+
['gl3d_streamtube-simple', require('@mocks/gl3d_streamtube-simple.json')],
57+
['glpolar_style', require('@mocks/glpolar_style.json')],
58+
];
59+
60+
var mapboxMockList = [
61+
['scattermapbox', require('@mocks/mapbox_bubbles-text.json')]
62+
];
63+
64+
module.exports = {
65+
svg: svgMockList,
66+
gl: glMockList,
67+
mapbox: mapboxMockList,
68+
all: svgMockList.concat(glMockList).concat(mapboxMockList)
69+
};

Diff for: ‎test/jasmine/bundle_tests/minified_bundle_test.js

+28
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/* global Plotly:false */
2+
3+
var MAPBOX_ACCESS_TOKEN = require('@build/credentials.json').MAPBOX_ACCESS_TOKEN;
4+
var mockLists = require('../assets/mock_lists');
5+
6+
// only needed for mapbox subplots
7+
var LONG_TIMEOUT_INTERVAL = 5 * jasmine.DEFAULT_TIMEOUT_INTERVAL;
8+
9+
describe('Test plotly.min.js', function() {
10+
'use strict';
11+
12+
var gd = document.createElement('div');
13+
document.body.appendChild(gd);
14+
15+
it('should expose Plotly global', function() {
16+
expect(window.Plotly).toBeDefined();
17+
});
18+
19+
Plotly.setPlotConfig({
20+
mapboxAccessToken: MAPBOX_ACCESS_TOKEN
21+
});
22+
23+
mockLists.all.forEach(function(mockSpec) {
24+
it('can plot "' + mockSpec[0] + '"', function(done) {
25+
Plotly.newPlot(gd, mockSpec[1]).catch(fail).then(done);
26+
}, LONG_TIMEOUT_INTERVAL);
27+
});
28+
});

Diff for: ‎test/jasmine/karma.conf.js

+4
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,10 @@ if(isFullSuite) {
255255
];
256256
delete func.defaultConfig.preprocessors[pathToCustomMatchers];
257257
break;
258+
case 'minified_bundle':
259+
func.defaultConfig.files.push(constants.pathToPlotlyDistMin);
260+
func.defaultConfig.preprocessors[testFileGlob] = ['browserify'];
261+
break;
258262
case 'ie9':
259263
// load ie9_mock.js before plotly.js+test bundle
260264
// to catch reference errors that could occur

Diff for: ‎test/jasmine/tests/plot_api_test.js

+9-64
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ var destroyGraphDiv = require('../assets/destroy_graph_div');
2020
var failTest = require('../assets/fail_test');
2121
var checkTicks = require('../assets/custom_assertions').checkTicks;
2222
var supplyAllDefaults = require('../assets/supply_defaults');
23+
var mockLists = require('../assets/mock_lists');
2324

2425
describe('Test plot api', function() {
2526
'use strict';
@@ -3230,63 +3231,6 @@ describe('Test plot api', function() {
32303231
.then(done);
32313232
});
32323233

3233-
var svgMockList = [
3234-
['1', require('@mocks/1.json')],
3235-
['4', require('@mocks/4.json')],
3236-
['5', require('@mocks/5.json')],
3237-
['10', require('@mocks/10.json')],
3238-
['11', require('@mocks/11.json')],
3239-
['17', require('@mocks/17.json')],
3240-
['21', require('@mocks/21.json')],
3241-
['22', require('@mocks/22.json')],
3242-
['airfoil', require('@mocks/airfoil.json')],
3243-
['annotations-autorange', require('@mocks/annotations-autorange.json')],
3244-
['axes_enumerated_ticks', require('@mocks/axes_enumerated_ticks.json')],
3245-
['axes_visible-false', require('@mocks/axes_visible-false.json')],
3246-
['bar_and_histogram', require('@mocks/bar_and_histogram.json')],
3247-
['basic_error_bar', require('@mocks/basic_error_bar.json')],
3248-
['binding', require('@mocks/binding.json')],
3249-
['cheater_smooth', require('@mocks/cheater_smooth.json')],
3250-
['finance_style', require('@mocks/finance_style.json')],
3251-
['geo_first', require('@mocks/geo_first.json')],
3252-
['layout_image', require('@mocks/layout_image.json')],
3253-
['layout-colorway', require('@mocks/layout-colorway.json')],
3254-
['polar_categories', require('@mocks/polar_categories.json')],
3255-
['polar_direction', require('@mocks/polar_direction.json')],
3256-
['range_selector_style', require('@mocks/range_selector_style.json')],
3257-
['range_slider_multiple', require('@mocks/range_slider_multiple.json')],
3258-
['sankey_energy', require('@mocks/sankey_energy.json')],
3259-
['scattercarpet', require('@mocks/scattercarpet.json')],
3260-
['shapes', require('@mocks/shapes.json')],
3261-
['splom_iris', require('@mocks/splom_iris.json')],
3262-
['table_wrapped_birds', require('@mocks/table_wrapped_birds.json')],
3263-
['ternary_fill', require('@mocks/ternary_fill.json')],
3264-
['text_chart_arrays', require('@mocks/text_chart_arrays.json')],
3265-
['transforms', require('@mocks/transforms.json')],
3266-
['updatemenus', require('@mocks/updatemenus.json')],
3267-
['violin_side-by-side', require('@mocks/violin_side-by-side.json')],
3268-
['world-cals', require('@mocks/world-cals.json')],
3269-
['typed arrays', {
3270-
data: [{
3271-
x: new Float32Array([1, 2, 3]),
3272-
y: new Float32Array([1, 2, 1])
3273-
}]
3274-
}]
3275-
];
3276-
3277-
var glMockList = [
3278-
['gl2d_heatmapgl', require('@mocks/gl2d_heatmapgl.json')],
3279-
['gl2d_line_dash', require('@mocks/gl2d_line_dash.json')],
3280-
['gl2d_parcoords_2', require('@mocks/gl2d_parcoords_2.json')],
3281-
['gl2d_pointcloud-basic', require('@mocks/gl2d_pointcloud-basic.json')],
3282-
['gl3d_annotations', require('@mocks/gl3d_annotations.json')],
3283-
['gl3d_set-ranges', require('@mocks/gl3d_set-ranges.json')],
3284-
['gl3d_world-cals', require('@mocks/gl3d_world-cals.json')],
3285-
['gl3d_cone-autorange', require('@mocks/gl3d_cone-autorange.json')],
3286-
['gl3d_streamtube-simple', require('@mocks/gl3d_streamtube-simple.json')],
3287-
['glpolar_style', require('@mocks/glpolar_style.json')],
3288-
];
3289-
32903234
// make sure we've included every trace type in this suite
32913235
var typesTested = {};
32923236
var itemType;
@@ -3385,24 +3329,25 @@ describe('Test plot api', function() {
33853329
.then(done);
33863330
}
33873331

3388-
svgMockList.forEach(function(mockSpec) {
3332+
mockLists.svg.forEach(function(mockSpec) {
33893333
it('can redraw "' + mockSpec[0] + '" with no changes as a noop (svg mocks)', function(done) {
33903334
_runReactMock(mockSpec, done);
33913335
});
33923336
});
33933337

3394-
glMockList.forEach(function(mockSpec) {
3338+
mockLists.gl.forEach(function(mockSpec) {
33953339
it('can redraw "' + mockSpec[0] + '" with no changes as a noop (gl mocks)', function(done) {
33963340
_runReactMock(mockSpec, done);
33973341
});
33983342
});
33993343

3400-
it('@noCI can redraw scattermapbox with no changes as a noop', function(done) {
3401-
Plotly.setPlotConfig({
3402-
mapboxAccessToken: require('@build/credentials.json').MAPBOX_ACCESS_TOKEN
3344+
mockLists.mapbox.forEach(function(mockSpec) {
3345+
it('@noCI can redraw "' + mockSpec[0] + '" with no changes as a noop (mapbpox mocks)', function(done) {
3346+
Plotly.setPlotConfig({
3347+
mapboxAccessToken: require('@build/credentials.json').MAPBOX_ACCESS_TOKEN
3348+
});
3349+
_runReactMock(mockSpec, done);
34033350
});
3404-
3405-
_runReactMock(['scattermapbox', require('@mocks/mapbox_bubbles-text.json')], done);
34063351
});
34073352

34083353
// since CI breaks up gl/svg types, and drops scattermapbox, this test won't work there

0 commit comments

Comments
 (0)