Skip to content

Better mapbox minification fix #2792

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 8 commits into from
Jul 11, 2018
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
2 changes: 1 addition & 1 deletion .circleci/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,13 @@ case $1 in

jasmine)
npm run test-jasmine -- --skip-tags=gl,noCI,flaky || EXIT_STATE=$?
npm run test-bundle || EXIT_STATE=$?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

which gives:

image

Copy link
Collaborator

Choose a reason for hiding this comment

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

⚖️ 👍
💃 still applies :)

exit $EXIT_STATE
;;

jasmine2)
retry npm run test-jasmine -- --tags=gl --skip-tags=noCI,flaky
retry npm run test-jasmine -- --tags=flaky --skip-tags=noCI
npm run test-bundle || EXIT_STATE=$?
exit $EXIT_STATE
;;

Expand Down
44 changes: 37 additions & 7 deletions tasks/test_bundle.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,46 @@
var path = require('path');
var exec = require('child_process').exec;
var glob = require('glob');
var runSeries = require('run-series');

var constants = require('./util/constants');
var common = require('./util/common');
var pathToJasmineBundleTests = path.join(constants.pathToJasmineBundleTests);

var pathToJasmineBundleTests = constants.pathToJasmineBundleTests;

/**
* Run all jasmine 'bundle' test in series
*
* To run specific bundle tests, use
*
* $ npm run test-jasmine -- --bundleTest=<name-of-suite>
*/
glob(pathToJasmineBundleTests + '/*.js', function(err, files) {
files.forEach(function(file) {
var baseName = path.basename(file);
var cmd = 'npm run test-jasmine -- --bundleTest=' + baseName;
var tasks = files.map(function(file) {
return function(cb) {
var cmd = [
'karma', 'start',
path.join(constants.pathToRoot, 'test', 'jasmine', 'karma.conf.js'),
'--bundleTest=' + path.basename(file),
'--nowatch'
].join(' ');

console.log('Running: ' + cmd);

exec(cmd, function(err) {
cb(null, err);
}).stdout.pipe(process.stdout);
};
});

runSeries(tasks, function(err, results) {
if(err) throw err;

var failed = results.filter(function(r) { return r; });

common.execCmd(cmd);
if(failed.length) {
console.log('\ntest-bundle summary:');
failed.forEach(function(r) { console.warn('- ' + r.cmd + ' failed'); });
console.log('');
process.exit(1);
}
});
});
4 changes: 2 additions & 2 deletions tasks/util/browserify_wrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ module.exports = function _bundle(pathToIndex, pathToBundle, opts, cb) {
}

var b = browserify(pathToIndex, browserifyOpts);
var pending = opts.pathToMinBundle ? 2 : 1;
var pending = pathToMinBundle ? 2 : 1;

function done() {
if(cb && --pending === 0) cb(null);
Expand All @@ -58,7 +58,7 @@ module.exports = function _bundle(pathToIndex, pathToBundle, opts, cb) {
}
});

if(opts.pathToMinBundle) {
if(pathToMinBundle) {
bundleStream
.pipe(minify(constants.uglifyOptions))
.pipe(fs.createWriteStream(pathToMinBundle))
Expand Down
13 changes: 10 additions & 3 deletions tasks/util/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,17 @@ module.exports = {
testContainerHome: '/var/www/streambed/image_server/plotly.js',

uglifyOptions: {
ecma: 5,
mangle: true,
// the compress flag break mapbox-gl,
// TODO find a way to only skip compression on mapbox-gl files
compress: false,
compress: {
// see full list of compress option
// https://github.com/fabiosantoscode/terser#compress-options
//
// need to turn off 'typeofs' to make mapbox-gl work in
// minified bundles, for more info see:
// https://github.com/plotly/plotly.js/issues/2787
typeofs: false
},
output: {
beautify: false,
ascii_only: true
Expand Down
69 changes: 69 additions & 0 deletions test/jasmine/assets/mock_lists.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
// list of mocks that should include *all* plotly.js trace modules

var svgMockList = [
['1', require('@mocks/1.json')],
['4', require('@mocks/4.json')],
['5', require('@mocks/5.json')],
['10', require('@mocks/10.json')],
['11', require('@mocks/11.json')],
['17', require('@mocks/17.json')],
['21', require('@mocks/21.json')],
['22', require('@mocks/22.json')],
['airfoil', require('@mocks/airfoil.json')],
['annotations-autorange', require('@mocks/annotations-autorange.json')],
['axes_enumerated_ticks', require('@mocks/axes_enumerated_ticks.json')],
['axes_visible-false', require('@mocks/axes_visible-false.json')],
['bar_and_histogram', require('@mocks/bar_and_histogram.json')],
['basic_error_bar', require('@mocks/basic_error_bar.json')],
['binding', require('@mocks/binding.json')],
['cheater_smooth', require('@mocks/cheater_smooth.json')],
['finance_style', require('@mocks/finance_style.json')],
['geo_first', require('@mocks/geo_first.json')],
['layout_image', require('@mocks/layout_image.json')],
['layout-colorway', require('@mocks/layout-colorway.json')],
['polar_categories', require('@mocks/polar_categories.json')],
['polar_direction', require('@mocks/polar_direction.json')],
['range_selector_style', require('@mocks/range_selector_style.json')],
['range_slider_multiple', require('@mocks/range_slider_multiple.json')],
['sankey_energy', require('@mocks/sankey_energy.json')],
['scattercarpet', require('@mocks/scattercarpet.json')],
['shapes', require('@mocks/shapes.json')],
['splom_iris', require('@mocks/splom_iris.json')],
['table_wrapped_birds', require('@mocks/table_wrapped_birds.json')],
['ternary_fill', require('@mocks/ternary_fill.json')],
['text_chart_arrays', require('@mocks/text_chart_arrays.json')],
['transforms', require('@mocks/transforms.json')],
['updatemenus', require('@mocks/updatemenus.json')],
['violin_side-by-side', require('@mocks/violin_side-by-side.json')],
['world-cals', require('@mocks/world-cals.json')],
['typed arrays', {
data: [{
x: new Float32Array([1, 2, 3]),
y: new Float32Array([1, 2, 1])
}]
}]
];

var glMockList = [
['gl2d_heatmapgl', require('@mocks/gl2d_heatmapgl.json')],
['gl2d_line_dash', require('@mocks/gl2d_line_dash.json')],
['gl2d_parcoords_2', require('@mocks/gl2d_parcoords_2.json')],
['gl2d_pointcloud-basic', require('@mocks/gl2d_pointcloud-basic.json')],
['gl3d_annotations', require('@mocks/gl3d_annotations.json')],
['gl3d_set-ranges', require('@mocks/gl3d_set-ranges.json')],
['gl3d_world-cals', require('@mocks/gl3d_world-cals.json')],
['gl3d_cone-autorange', require('@mocks/gl3d_cone-autorange.json')],
['gl3d_streamtube-simple', require('@mocks/gl3d_streamtube-simple.json')],
['glpolar_style', require('@mocks/glpolar_style.json')],
];

var mapboxMockList = [
['scattermapbox', require('@mocks/mapbox_bubbles-text.json')]
];

module.exports = {
svg: svgMockList,
gl: glMockList,
mapbox: mapboxMockList,
all: svgMockList.concat(glMockList).concat(mapboxMockList)
};
28 changes: 28 additions & 0 deletions test/jasmine/bundle_tests/minified_bundle_test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/* global Plotly:false */

var MAPBOX_ACCESS_TOKEN = require('@build/credentials.json').MAPBOX_ACCESS_TOKEN;
var mockLists = require('../assets/mock_lists');

// only needed for mapbox subplots
var LONG_TIMEOUT_INTERVAL = 5 * jasmine.DEFAULT_TIMEOUT_INTERVAL;

describe('Test plotly.min.js', function() {
'use strict';

var gd = document.createElement('div');
document.body.appendChild(gd);

it('should expose Plotly global', function() {
expect(window.Plotly).toBeDefined();
});

Plotly.setPlotConfig({
mapboxAccessToken: MAPBOX_ACCESS_TOKEN
});

mockLists.all.forEach(function(mockSpec) {
it('can plot "' + mockSpec[0] + '"', function(done) {
Plotly.newPlot(gd, mockSpec[1]).catch(fail).then(done);
}, LONG_TIMEOUT_INTERVAL);
});
});
4 changes: 4 additions & 0 deletions test/jasmine/karma.conf.js
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,10 @@ if(isFullSuite) {
];
delete func.defaultConfig.preprocessors[pathToCustomMatchers];
break;
case 'minified_bundle':
func.defaultConfig.files.push(constants.pathToPlotlyDistMin);
func.defaultConfig.preprocessors[testFileGlob] = ['browserify'];
break;
case 'ie9':
// load ie9_mock.js before plotly.js+test bundle
// to catch reference errors that could occur
Expand Down
73 changes: 9 additions & 64 deletions test/jasmine/tests/plot_api_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ var destroyGraphDiv = require('../assets/destroy_graph_div');
var failTest = require('../assets/fail_test');
var checkTicks = require('../assets/custom_assertions').checkTicks;
var supplyAllDefaults = require('../assets/supply_defaults');
var mockLists = require('../assets/mock_lists');

describe('Test plot api', function() {
'use strict';
Expand Down Expand Up @@ -3230,63 +3231,6 @@ describe('Test plot api', function() {
.then(done);
});

var svgMockList = [
['1', require('@mocks/1.json')],
['4', require('@mocks/4.json')],
['5', require('@mocks/5.json')],
['10', require('@mocks/10.json')],
['11', require('@mocks/11.json')],
['17', require('@mocks/17.json')],
['21', require('@mocks/21.json')],
['22', require('@mocks/22.json')],
['airfoil', require('@mocks/airfoil.json')],
['annotations-autorange', require('@mocks/annotations-autorange.json')],
['axes_enumerated_ticks', require('@mocks/axes_enumerated_ticks.json')],
['axes_visible-false', require('@mocks/axes_visible-false.json')],
['bar_and_histogram', require('@mocks/bar_and_histogram.json')],
['basic_error_bar', require('@mocks/basic_error_bar.json')],
['binding', require('@mocks/binding.json')],
['cheater_smooth', require('@mocks/cheater_smooth.json')],
['finance_style', require('@mocks/finance_style.json')],
['geo_first', require('@mocks/geo_first.json')],
['layout_image', require('@mocks/layout_image.json')],
['layout-colorway', require('@mocks/layout-colorway.json')],
['polar_categories', require('@mocks/polar_categories.json')],
['polar_direction', require('@mocks/polar_direction.json')],
['range_selector_style', require('@mocks/range_selector_style.json')],
['range_slider_multiple', require('@mocks/range_slider_multiple.json')],
['sankey_energy', require('@mocks/sankey_energy.json')],
['scattercarpet', require('@mocks/scattercarpet.json')],
['shapes', require('@mocks/shapes.json')],
['splom_iris', require('@mocks/splom_iris.json')],
['table_wrapped_birds', require('@mocks/table_wrapped_birds.json')],
['ternary_fill', require('@mocks/ternary_fill.json')],
['text_chart_arrays', require('@mocks/text_chart_arrays.json')],
['transforms', require('@mocks/transforms.json')],
['updatemenus', require('@mocks/updatemenus.json')],
['violin_side-by-side', require('@mocks/violin_side-by-side.json')],
['world-cals', require('@mocks/world-cals.json')],
['typed arrays', {
data: [{
x: new Float32Array([1, 2, 3]),
y: new Float32Array([1, 2, 1])
}]
}]
];

var glMockList = [
['gl2d_heatmapgl', require('@mocks/gl2d_heatmapgl.json')],
['gl2d_line_dash', require('@mocks/gl2d_line_dash.json')],
['gl2d_parcoords_2', require('@mocks/gl2d_parcoords_2.json')],
['gl2d_pointcloud-basic', require('@mocks/gl2d_pointcloud-basic.json')],
['gl3d_annotations', require('@mocks/gl3d_annotations.json')],
['gl3d_set-ranges', require('@mocks/gl3d_set-ranges.json')],
['gl3d_world-cals', require('@mocks/gl3d_world-cals.json')],
['gl3d_cone-autorange', require('@mocks/gl3d_cone-autorange.json')],
['gl3d_streamtube-simple', require('@mocks/gl3d_streamtube-simple.json')],
['glpolar_style', require('@mocks/glpolar_style.json')],
];

// make sure we've included every trace type in this suite
var typesTested = {};
var itemType;
Expand Down Expand Up @@ -3385,24 +3329,25 @@ describe('Test plot api', function() {
.then(done);
}

svgMockList.forEach(function(mockSpec) {
mockLists.svg.forEach(function(mockSpec) {
it('can redraw "' + mockSpec[0] + '" with no changes as a noop (svg mocks)', function(done) {
_runReactMock(mockSpec, done);
});
});

glMockList.forEach(function(mockSpec) {
mockLists.gl.forEach(function(mockSpec) {
it('can redraw "' + mockSpec[0] + '" with no changes as a noop (gl mocks)', function(done) {
_runReactMock(mockSpec, done);
});
});

it('@noCI can redraw scattermapbox with no changes as a noop', function(done) {
Plotly.setPlotConfig({
mapboxAccessToken: require('@build/credentials.json').MAPBOX_ACCESS_TOKEN
mockLists.mapbox.forEach(function(mockSpec) {
it('@noCI can redraw "' + mockSpec[0] + '" with no changes as a noop (mapbpox mocks)', function(done) {
Plotly.setPlotConfig({
mapboxAccessToken: require('@build/credentials.json').MAPBOX_ACCESS_TOKEN
});
_runReactMock(mockSpec, done);
});

_runReactMock(['scattermapbox', require('@mocks/mapbox_bubbles-text.json')], done);
});

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