-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Custom bundle script details #5527
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
Changes from 2 commits
9fe3255
ca4d557
ba00b93
cbb431e
5aa2234
7de7be9
df8005a
efa9357
9ca5d44
9d34652
0869970
7d0145a
fa35c86
65efc42
b05beef
fc25200
bcb7967
9411d7f
1e21747
34c09c3
6974914
4b0f1da
90bf969
3693985
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
var path = require('path'); | ||
var minimist = require('minimist'); | ||
var runSeries = require('run-series'); | ||
var prependFile = require('prepend-file'); | ||
|
||
|
@@ -7,39 +8,97 @@ var common = require('./util/common'); | |
var _bundle = require('./util/browserify_wrapper'); | ||
|
||
var header = constants.licenseDist + '\n'; | ||
var allTransforms = constants.allTransforms; | ||
var allTraces = constants.allTraces; | ||
var mainIndex = constants.mainIndex; | ||
|
||
var argv = process.argv; | ||
function isFalse(a) { | ||
return ( | ||
a === 'none' || | ||
a === 'false' | ||
); | ||
} | ||
|
||
function inputBoolean(a, dflt) { | ||
return !a ? dflt : !isFalse(a); | ||
} | ||
|
||
function inputArray(a, dflt) { | ||
dflt = dflt.slice(); | ||
|
||
return ( | ||
isFalse(a) ? [] : | ||
!a || a === 'all' ? dflt : | ||
a.split(',') | ||
); | ||
} | ||
|
||
if(argv.length > 2) { | ||
if(process.argv.length > 2) { | ||
// command line | ||
|
||
var traceList = ['scatter']; // added by default | ||
var name; | ||
for(var i = 2; i < argv.length; i++) { | ||
var a = argv[i]; | ||
var args = minimist(process.argv.slice(2), {}); | ||
|
||
// parse arguments | ||
var out = args.out ? args.out : 'custom'; | ||
var traces = inputArray(args.traces, allTraces); | ||
var transforms = inputArray(args.transforms, allTransforms); | ||
var calendars = inputBoolean(args.calendars, true); | ||
var keepindex = inputBoolean(args.keepindex, false); | ||
var sourcemaps = inputBoolean(args.sourcemaps, false); | ||
archmoj marked this conversation as resolved.
Show resolved
Hide resolved
|
||
var unminified = inputBoolean(args.unminified, false); | ||
|
||
var i, t; | ||
|
||
var traceList = ['scatter']; // added by default | ||
for(i = 0; i < traces.length; i++) { | ||
t = traces[i]; | ||
if( | ||
allTraces.indexOf(a) !== -1 && // requested | ||
traceList.indexOf(a) === -1 // not added before | ||
traceList.indexOf(t) === -1 // not added before | ||
) { | ||
traceList.push(a); | ||
if(allTraces.indexOf(t) === -1) { | ||
console.error(t, 'is not a valid trace!', 'Valid traces are:', allTraces); | ||
} else { | ||
traceList.push(t); | ||
} | ||
} | ||
if(a.indexOf('--name=') === 0) name = a.replace('--name=', ''); | ||
} | ||
if(!name) name = 'custom'; | ||
traceList = traceList.sort(); | ||
|
||
var transformList = []; | ||
for(i = 0; i < transforms.length; i++) { | ||
t = transforms[i]; | ||
if( | ||
transformList.indexOf(t) === -1 // not added before | ||
) { | ||
if(allTransforms.indexOf(t) === -1) { | ||
console.error(t, 'is not a valid transform!', 'Valid transforms are:', allTransforms); | ||
} else { | ||
transformList.push(t); | ||
} | ||
} | ||
} | ||
transformList = transformList.sort(); | ||
|
||
var opts = { | ||
traceList: traceList, | ||
name: name, | ||
transformList: transformList, | ||
calendars: calendars, | ||
sourcemaps: sourcemaps, | ||
|
||
index: path.join(constants.pathToBuild, 'index-' + name + '.js'), | ||
dist: path.join(constants.pathToDist, 'plotly-' + name + '.js'), | ||
distMin: path.join(constants.pathToDist, 'plotly-' + name + '.min.js') | ||
name: out, | ||
index: path.join(constants.pathToLib, 'index-' + out + '.js') | ||
}; | ||
|
||
if(unminified) { | ||
opts.dist = path.join(constants.pathToDist, 'plotly-' + out + '.js'); | ||
} else { | ||
opts.distMin = path.join(constants.pathToDist, 'plotly-' + out + '.min.js'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To avoid name conflicts (let's say one uses e.g. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On the other hand folks making custom bundles will likely be used to our partial bundles already, so may be tripped up by the switch from |
||
} | ||
|
||
if(!keepindex) { | ||
opts.deleteIndex = true; | ||
} | ||
|
||
console.log(opts); | ||
|
||
var tasks = []; | ||
|
@@ -58,40 +117,55 @@ function partialBundle(tasks, opts) { | |
var dist = opts.dist; | ||
var distMin = opts.distMin; | ||
var traceList = opts.traceList; | ||
var transformList = opts.transformList; | ||
var calendars = opts.calendars; | ||
var sourcemaps = opts.sourcemaps; | ||
var deleteIndex = opts.deleteIndex; | ||
|
||
tasks.push(function(done) { | ||
var partialIndex = mainIndex; | ||
allTraces.forEach(function(trace) { | ||
if(traceList.indexOf(trace) === -1) { | ||
var WHITESPACE_BEFORE = '\\s*'; | ||
// remove require | ||
var newCode = partialIndex.replace( | ||
new RegExp( | ||
WHITESPACE_BEFORE + | ||
'require\\(\'\\./' + trace + '\'\\),', | ||
'g'), '' | ||
); | ||
|
||
// test removal | ||
if(newCode === partialIndex) throw 'Unable to find and drop require for trace: "' + trace + '"'; | ||
|
||
partialIndex = newCode; | ||
|
||
var all = ['calendars'].concat(allTransforms).concat(allTraces); | ||
var includes = (calendars ? ['calendars'] : []).concat(transformList).concat(traceList); | ||
var excludes = all.filter(function(e) { return includes.indexOf(e) === -1; }); | ||
|
||
excludes.forEach(function(t) { | ||
var WHITESPACE_BEFORE = '\\s*'; | ||
// remove require | ||
var newCode = partialIndex.replace( | ||
new RegExp( | ||
WHITESPACE_BEFORE + | ||
'require\\(\'\\./' + t + '\'\\)' + | ||
(t === 'calendars' ? '' : ','), // there is no comma after calendars require | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Feels a bit fragile - can we either just make the comma optional in the pattern, or add the comma in |
||
'g'), '' | ||
); | ||
|
||
// test removal | ||
if(newCode === partialIndex) { | ||
console.error('Unable to find and drop require for ' + t); | ||
throw 'Error generating index for partial bundle!'; | ||
} | ||
|
||
partialIndex = newCode; | ||
}); | ||
|
||
common.writeFile(index, partialIndex, done); | ||
}); | ||
|
||
tasks.push(function(done) { | ||
_bundle(index, dist, { | ||
var bundleOpts = { | ||
debug: sourcemaps, | ||
deleteIndex: deleteIndex, | ||
standalone: 'Plotly', | ||
pathToMinBundle: distMin | ||
}, function() { | ||
}; | ||
|
||
_bundle(index, dist, bundleOpts, function() { | ||
var headerDist = header.replace('plotly.js', 'plotly.js (' + name + ')'); | ||
var headerDistMin = header.replace('plotly.js', 'plotly.js (' + name + ' - minified)'); | ||
|
||
prependFile(dist, headerDist, common.throwOnError); | ||
prependFile(distMin, headerDistMin, common.throwOnError); | ||
if(dist) prependFile(dist, headerDist, common.throwOnError); | ||
if(distMin) prependFile(distMin, headerDistMin, common.throwOnError); | ||
|
||
done(); | ||
}); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
presumably at some point we will make more of the components registered in core optional. In order to support this in a backward-compatible way, perhaps we can make an option like
skipcomponents=calendars
, to later expand to allow skipping other components?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also thought about this a bit. But I think another option that we want to provide in future is which calendar(s) to add. That can explain why
calendars
logic here is quite similar totransforms
andtraces
. For the moment it is handled by a boolean; but it could be expanded to a list.Perhaps to avoid confusion we can move
calendars
code from./components/calendars
to./calendars
now or once we split it?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed on slack, for now let's drop the calendars option - always include calendars in every bundle - until we can ensure that unrelated functionality like
connectgaps
will not break as in #4883.At ~81kB minified and rarely used, it would be very nice to make these optional eventually, and you're right to want flexibility even when world calendars are requested to only include the necessary ones, not all of them