Skip to content
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

Uglify sourcemaps support #617

Merged
merged 21 commits into from
Feb 21, 2018
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
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
4 changes: 1 addition & 3 deletions src/Bundler.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,7 @@ class Bundler extends EventEmitter {
hmrPort: options.hmrPort || 0,
rootDir: Path.dirname(this.mainFile),
sourceMaps:
typeof options.sourceMaps === 'boolean'
? options.sourceMaps
: !isProduction,
typeof options.sourceMaps === 'boolean' ? options.sourceMaps : true,
hmrHostname: options.hmrHostname || '',
detailedReport: options.detailedReport || false
};
Expand Down
89 changes: 85 additions & 4 deletions src/SourceMap.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,12 +137,12 @@ class SourceMap {

async extendSourceMap(original, extension) {
if (!(extension instanceof SourceMap)) {
throw new Error(
'[SOURCEMAP] Type of extension should be a SourceMap instance!'
);
extension = await new SourceMap().addMap(extension);
}
if (!(original instanceof SourceMap)) {
original = await this.getConsumer(original);
}

original = await this.getConsumer(original);
extension.eachMapping(mapping => {
let originalMapping = original.originalPositionFor({
line: mapping.original.line,
Expand Down Expand Up @@ -182,6 +182,87 @@ class SourceMap {
return this;
}

findClosest(line, column, key = 'original') {
if (line < 1) {
throw new Error('Line numbers must be >= 1');
}

if (column < 0) {
throw new Error('Column numbers must be >= 0');
}

if (this.mappings.length < 1) {
return undefined;
}

let startIndex = 0;
let stopIndex = this.mappings.length - 1;
let middleIndex = Math.floor((stopIndex + startIndex) / 2);

while (
this.mappings[middleIndex][key].line !== line &&
startIndex < stopIndex
) {
if (line < this.mappings[middleIndex][key].line) {
stopIndex = middleIndex - 1;
} else if (line > this.mappings[middleIndex][key].line) {
startIndex = middleIndex + 1;
}
middleIndex = Math.floor((stopIndex + startIndex) / 2);
}

if (this.mappings[middleIndex][key].line !== line) {
return middleIndex;
}

while (
middleIndex >= 1 &&
this.mappings[middleIndex - 1][key].line === line
) {
middleIndex--;
}
while (
middleIndex < this.mappings.length - 1 &&
this.mappings[middleIndex + 1][key].line === line &&
column > this.mappings[middleIndex][key].column
) {
middleIndex++;
}
return middleIndex;
}

originalPositionFor(generatedPosition) {
let index = this.findClosest(
generatedPosition.line,
generatedPosition.column,
'generated'
);
return {
source: this.mappings[index].source,
name: this.mappings[index].name,
line: this.mappings[index].original.line,
column: this.mappings[index].original.column
};
}

generatedPositionFor(originalPosition) {
let index = this.findClosest(
originalPosition.line,
originalPosition.column,
'original'
);
return {
source: this.mappings[index].source,
name: this.mappings[index].name,
line: this.mappings[index].generated.line,
column: this.mappings[index].generated.column
};
}

sourceContentFor(fileName) {
return this.sources[fileName];
}

offset(lineOffset = 0, columnOffset = 0) {
this.mappings.map(mapping => {
mapping.generated.line = mapping.generated.line + lineOffset;
Expand Down
1 change: 1 addition & 0 deletions src/cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ program
)
.option('--no-minify', 'disable minification')
.option('--no-cache', 'disable the filesystem cache')
.option('--no-source-maps', 'disable sourcemaps')
.option(
'-t, --target <target>',
'set the runtime environment, either "node", "browser" or "electron". defaults to "browser"',
Expand Down
25 changes: 24 additions & 1 deletion src/transforms/uglify.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
const {minify} = require('uglify-es');
const SourceMap = require('../SourceMap');
const logger = require('../Logger');

module.exports = async function(asset) {
await asset.parseIfNeeded();
Expand All @@ -11,18 +13,39 @@ module.exports = async function(asset) {
warnings: true,
mangle: {
toplevel: true
}
},
sourceMap: asset.options.sourceMaps ? {filename: asset.relativeName} : false
};

if (customConfig) {
options = Object.assign(options, customConfig);
}

let result = minify(code, options);

if (result.error) {
throw result.error;
}

if (result.map) {
result.map = await new SourceMap().addMap(JSON.parse(result.map));
Copy link
Member

Choose a reason for hiding this comment

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

I think it's possible to get the map without converting to JSON first. https://github.com/mishoo/UglifyJS2/blob/master/lib/sourcemap.js#L94

Might be faster - maybe you could even access the original mappings from there so you don't need to decode it.

if (asset.sourceMap) {
asset.sourceMap = await new SourceMap().extendSourceMap(
asset.sourceMap,
result.map
);
} else {
asset.sourceMap = result.map;
}
}

// Log all warnings
Copy link
Member

Choose a reason for hiding this comment

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

I'd rather not log all of these. Uglify logs a lot of warnings and they are pretty much useless. I removed this in master already.

if (result.warnings) {
result.warnings.forEach(warning => {
logger.warn('[uglify] ' + warning);
});
}

// babel-generator did our code generation for us, so remove the old AST
asset.ast = null;
asset.outputCode = result.code;
Expand Down
3 changes: 3 additions & 0 deletions test/css.js
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,9 @@ describe('css', function() {
assets: ['index.css'],
childBundles: []
},
{
type: 'map'
},
{
type: 'woff2',
assets: ['test.woff2'],
Expand Down
5 changes: 5 additions & 0 deletions test/integration/sourcemap-nested-minified/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
const local = require('./local');

module.exports = function() {
return local.a + local.b;
}
4 changes: 4 additions & 0 deletions test/integration/sourcemap-nested-minified/local.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
const util = require('./utils/util');

exports.a = 5;
exports.b = util.count(4, 5);
3 changes: 3 additions & 0 deletions test/integration/sourcemap-nested-minified/utils/util.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
exports.count = function(a, b) {
return a + b;
}
32 changes: 32 additions & 0 deletions test/sourcemaps.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,4 +116,36 @@ describe('sourcemaps', function() {
assert.equal(typeof output, 'function');
assert.equal(output(), 14);
});

it('should create a valid sourcemap for a minified js bundle with requires', async function() {
let b = await bundle(
__dirname + '/integration/sourcemap-nested-minified/index.js',
{
minify: true
}
);

assertBundleTree(b, {
name: 'index.js',
assets: ['index.js', 'local.js', 'util.js'],
childBundles: [
{
name: 'index.map',
type: 'map'
}
]
});

let raw = fs
.readFileSync(path.join(__dirname, '/dist/index.js'))
.toString();
let map = fs
.readFileSync(path.join(__dirname, '/dist/index.map'))
.toString();
mapValidator(raw, map);

let output = run(b);
assert.equal(typeof output, 'function');
assert.equal(output(), 14);
});
});
2 changes: 1 addition & 1 deletion test/typescript.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ describe('typescript', function() {
);

assert.equal(b.assets.size, 2);
assert.equal(b.childBundles.size, 0);
assert.equal(b.childBundles.size, 1);

let output = run(b);
assert.equal(typeof output.count, 'function');
Expand Down