Add an unminified production build to the NPM package #7403
Conversation
package.json
Outdated
| @@ -110,6 +110,7 @@ | |||
| "scripts": { | |||
| "build-dev": "rollup -c --environment BUILD:dev", | |||
| "watch-dev": "rollup -c --environment BUILD:dev --watch", | |||
| "build-alt": "rollup -c --environment BUILD:production-unminified", | |||
There was a problem hiding this comment.
Just a short name for the package script (which shouldn't be called directly anyway), "alternative" build. Didn't want to name it build-production-unminified.
rollup.config.js
Outdated
| const outputFile = production ? 'dist/mapbox-gl.js' : 'dist/mapbox-gl-dev.js'; | ||
| const {BUILD} = process.env; | ||
| const minified = BUILD === 'production'; | ||
| const production = BUILD === 'production' || BUILD === 'production-unminified'; |
There was a problem hiding this comment.
Can we remove the conflation of debug/release and minification? We could have two separate variables: BUILD controls dev vs. release, while UGLIFY (defaulting to true) controls minification
|
@kkaefer addressed your comments. |
mourner
force-pushed
the
build-prod-unminified
branch
from
8936d97 to
30a842c
Compare
rollup.config.js
Outdated
| const production = BUILD === 'production'; | ||
| const outputFile = | ||
| production && minified ? 'dist/mapbox-gl.js' : | ||
| production ? 'dist/mapbox-gl-unminified.js' : 'dist/mapbox-gl-dev.js'; |
There was a problem hiding this comment.
This still doesn't account for the matrix of BUILD/MINIFY yet :) How about
const suffix = (production ? '' : '-dev') + (minified ? '' : '-unminified');
const outputFile = 'dist/mapbox-gl' + suffix + '.js';There was a problem hiding this comment.
@kkaefer I wouldn't want to change names of existing builds. This suggestion would rename mapbox-gl-dev.js to mapbox-gl-dev-unminified.js.
There was a problem hiding this comment.
Let's reverse the MINIFY flag and default to true, then specify MINIFY:false for the non-minified production build
There was a problem hiding this comment.
@kkaefer and rename the current mapbox-gl.js build to mapbox-gl-min.js?
There was a problem hiding this comment.
No, the suffix only gets added for unminified builds
There was a problem hiding this comment.
@kkaefer so it would still be mapbox-gl-dev-unminified.js? I'm confused.
There was a problem hiding this comment.
Sorry for the confusion; I didn't realize that the dev version is unminified. What I was trying to get at is that we have 4 possible combinations of BUILD and MINIFY, but we're only producing 3 different file names; the one for BUILD:development,MINIFY:true produces the same output file.
There was a problem hiding this comment.
Since it doesn't make sense to have a minified dev build anyway, I don't think we need to worry about it. I added another commit that hopefully makes build naming logic clearer. Feel free to add any changes you feel are needed here and I'll review, and otherwise we can merge.
Provides an alternative build in the published NPM package which is like the production build but unminified (e.g. for use with custom minifiers).
Launch Checklist
write tests for all new functionalitydocument any changes to public APIsnot sure if we should document anywherepost benchmark scores