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

Stop adding line number comments to linked JavaScript files. #9323

Merged
merged 2 commits into from
Nov 8, 2017
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
1 change: 0 additions & 1 deletion tools/isobuild/compiler-plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -1301,7 +1301,6 @@ export class PackageSourceBatch {
// XXX report an error if there is a package called global-imports
importStubServePath: isApp && '/packages/global-imports.js',
includeSourceMapInstructions: isWeb,
noLineNumbers: !isWeb
};

const fileHashes = [];
Expand Down
6 changes: 0 additions & 6 deletions tools/isobuild/compiler.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,6 @@ compiler.compile = Profile(function (packageSource, options) {
"` in package `" + packageSource.name + "`",
rootPath: packageSource.sourceRoot
}, function () {
// XXX we should probably also pass options.noLineNumbers into
// buildJsImage so it can pass it back to its call to
// compiler.compile
var buildResult = bundler.buildJsImage({
name: info.name,
packageMap: packageMap,
Expand Down Expand Up @@ -186,7 +183,6 @@ compiler.compile = Profile(function (packageSource, options) {
sourceArch: architecture,
isopackCache: isopackCache,
nodeModulesPath: nodeModulesPath,
noLineNumbers: options.noLineNumbers
});

_.extend(pluginProviderPackageNames,
Expand Down Expand Up @@ -345,8 +341,6 @@ var compileUnibuild = Profile(function (options) {
const inputSourceArch = options.sourceArch;
const isopackCache = options.isopackCache;
const nodeModulesPath = options.nodeModulesPath;
const noLineNumbers = options.noLineNumbers;

const isApp = ! inputSourceArch.pkg.name;
const resources = [];
const pluginProviderPackageNames = {};
Expand Down
3 changes: 0 additions & 3 deletions tools/isobuild/isopack-cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,6 @@ export class IsopackCache {
// Map from package name to Isopack.
self._isopacks = Object.create(null);

self._noLineNumbers = !! options.noLineNumbers;

self._lintLocalPackages = !! options.lintLocalPackages;
self._lintPackageWithSourceRoot = options.lintPackageWithSourceRoot;

Expand Down Expand Up @@ -362,7 +360,6 @@ export class IsopackCache {
isopack = compiler.compile(packageInfo.packageSource, {
packageMap: self._packageMap,
isopackCache: self,
noLineNumbers: self._noLineNumbers,
includeCordovaUnibuild: self._includeCordovaUnibuild,
includePluginProviderPackageMap: true,
pluginCacheDir: pluginCacheDir
Expand Down
97 changes: 5 additions & 92 deletions tools/isobuild/linker.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ var Module = function (options) {
self.meteorInstallOptions = options.meteorInstallOptions;
self.useGlobalNamespace = options.useGlobalNamespace;
self.combinedServePath = options.combinedServePath;
self.noLineNumbers = options.noLineNumbers;
};

_.extend(Module.prototype, {
Expand Down Expand Up @@ -188,7 +187,6 @@ _.extend(Module.prototype, {

chunks.push(file.getPrelinkedOutput({
sourceWidth: sourceWidth,
noLineNumbers: self.noLineNumbers
}));

++fileCount;
Expand Down Expand Up @@ -255,7 +253,6 @@ _.extend(Module.prototype, {
const { code: source, map } =
file.getPrelinkedOutput({
sourceWidth: sourceWidth,
noLineNumbers: this.noLineNumbers
}).toStringWithSourceMap({
file: servePath,
});
Expand Down Expand Up @@ -340,7 +337,6 @@ _.extend(Module.prototype, {

chunks.push(t.getPrelinkedOutput({
sourceWidth,
noLineNumbers: self.noLineNumbers
}));

} else if (_.isObject(t)) {
Expand Down Expand Up @@ -429,7 +425,6 @@ _.extend(Module.prototype, {
if (file.bare) {
chunks.push("\n", file.getPrelinkedOutput({
sourceWidth,
noLineNumbers: this.noLineNumbers
}));
} else if (moduleCount > 0 && ! file.lazy) {
eagerModuleFiles.push(file);
Expand Down Expand Up @@ -695,104 +690,29 @@ _.extend(File.prototype, {
// - preserveLineNumbers: if true, decorate minimally so that line
// numbers don't change between input and output. In this case,
// sourceWidth is ignored.
// - noLineNumbers: We still include the banners and such, but
// no line number suffix.
// - sourceWidth: width in columns to use for the source code
//
// Returns a SourceNode.
getPrelinkedOutput: Profile("linker File#getPrelinkedOutput", function (options) {
var self = this;
var width = options.sourceWidth || 70;
var bannerWidth = width + 3;
var noLineNumbers = options.noLineNumbers;
var preserveLineNumbers = options.preserveLineNumbers;
var result;

if (self.sourceMap) {
// If we have a source map, and options.noLineNumbers was not
// specified, it is important to annotate line numbers using that
// source map, since not all browsers support source maps.
if (typeof noLineNumbers === "undefined") {
noLineNumbers = false;
}

// Honoring options.preserveLineNumbers is likely impossible if we
// have a source map, since self.source has probably already been
// transformed in a way that does not preserve line numbers. That's
// ok, though, because we have a source map, and we also annotate
// line numbers using comments (see above), just in case source maps
// are not supported.
preserveLineNumbers = false;

} else if (preserveLineNumbers) {
// If we don't have a source map, and we're supposed to be preserving line
// numbers (ie, we are not linking multiple files into one file, because
// we're the app), then we can get away without annotating line numbers
// (or making a source map), because they won't add any helpful
// information.
noLineNumbers = true;
}

let consumer;
let lines;

if (self.sourceMap) {
result = {
code: self.source,
map: self.sourceMap
};

consumer = new sourcemap.SourceMapConsumer(result.map);

} else {
result = {
code: self.source,
map: null,
};

// Generating line number comments for really big files is not
// really worth it when there's no meaningful self.sourceMap.
if (! noLineNumbers && result.code.length < 500000) {
consumer = {
originalPositionFor(pos) {
return pos;
}
};
}
}

if (consumer && ! noLineNumbers) {
var padding = bannerPadding(bannerWidth);

// We might have already done this split above.
lines = lines || result.code.split(/\r?\n/);

// Use the SourceMapConsumer object to compute the original line
// number for each line of result.code.
for (var i = 0, lineCount = lines.length; i < lineCount; ++i) {
var line = lines[i];
var len = line.length;
if (len < width &&
line[len - 1] !== "\\") {
var pos = consumer.originalPositionFor({
line: i + 1,
column: 0
});

if (pos) {
line += padding.slice(len, width) + " //";
// Not all source maps define a mapping for every line in the
// output. This is perfectly normal.
if (typeof pos.line === "number") {
line += " " + pos.line;
}
lines[i] = line;
}
}
}

result.code = lines.join("\n");
}
const result = {
code: self.source,
map: self.sourceMap || null,
};

var chunks = [];
var pathNoSlash = convertColons(self.servePath.replace(/^\//, ""));
Expand Down Expand Up @@ -827,11 +747,7 @@ _.extend(File.prototype, {

let chunk = result.code;

if (consumer instanceof sourcemap.SourceMapConsumer) {
chunk = sourcemap.SourceNode.fromStringWithSourceMap(
result.code, consumer);

} else if (consumer && result.map) {
if (result.map) {
chunk = sourcemap.SourceNode.fromStringWithSourceMap(
result.code,
new sourcemap.SourceMapConsumer(result.map),
Expand Down Expand Up @@ -943,7 +859,6 @@ export var prelink = Profile("linker.prelink", function (options) {
var module = new Module({
name: options.name,
combinedServePath: options.combinedServePath,
noLineNumbers: options.noLineNumbers
});

_.each(options.inputFiles, function (inputFile) {
Expand Down Expand Up @@ -1100,7 +1015,6 @@ export var fullLink = Profile("linker.fullLink", function (inputFiles, {
// True if JS files with source maps should have a comment explaining
// how to use them in a browser.
includeSourceMapInstructions,
noLineNumbers,
}) {
buildmessage.assertInJob();

Expand All @@ -1109,7 +1023,6 @@ export var fullLink = Profile("linker.fullLink", function (inputFiles, {
meteorInstallOptions,
useGlobalNamespace,
combinedServePath,
noLineNumbers
});

_.each(inputFiles, file => module.addFile(file));
Expand Down