- switch from X-SourceMap header to //@ comment

- specify sourceMapUrl in browser manifest
- don't make source maps for app pure-JS files
- make URLs happy
- break traceback beauty
This commit is contained in:
David Glasser
2013-07-11 16:59:46 -07:00
parent 3205e33537
commit 4308b7c063
4 changed files with 85 additions and 84 deletions

View File

@@ -215,8 +215,8 @@ var runWebAppServer = function () {
next();
}
});
// Parse the query string into res.query. Used to detect sourcemap=1 and by
// oauth_server.
// Parse the query string into res.query. Used by oauth_server, but it's
// generally pretty handy..
app.use(connect.query());
// Auto-compress any json, javascript, or text.
@@ -225,31 +225,19 @@ var runWebAppServer = function () {
var staticFiles = {};
_.each(clientJson.manifest, function (item) {
if (item.url && item.where === "client") {
var staticFile = {
staticFiles[url.parse(item.url).pathname] = {
path: item.path,
cacheable: item.cacheable
};
// Serve the source map too, under a hashed URL. Note that the hash is
// based on item.url which contains the file's hash, so this should change
// when the file changes and thus be cacheable. The URL ends with a slash,
// so that source files referenced from the source map with relative URLs
// are resolved under it.
// Serve the source map too, under the specified URL. We assume all source
// maps are cacheable.
if (item.sourceMap) {
var sourceMapUrl = item.url;
var argument = "sourcemap=y";
if (sourceMapUrl.indexOf('?') === -1)
sourceMapUrl += '?' + argument;
else
sourceMapUrl += '&' + argument;
// Send the SourceMap header when the source file is served.
staticFile.urlToSourceMap = sourceMapUrl;
// Register the source map itself to be served.
staticFile.sourceMapFile = item.sourceMap;
staticFiles[url.parse(item.sourceMapUrl).pathname] = {
path: item.sourceMap,
cacheable: true
};
}
staticFiles[url.parse(item.url).pathname] = staticFile;
}
});
@@ -279,18 +267,6 @@ var runWebAppServer = function () {
var info = staticFiles[pathname];
var fileToServe, urlToSourceMap;
if (req.query.sourcemap === 'y') {
// This is a request for a sourcemap.
fileToServe = info.sourceMapFile;
// Source maps don't have source maps.
urlToSourceMap = null;
} else {
// This is not a request for a sourcemap.
fileToServe = info.path;
urlToSourceMap = info.urlToSourceMap;
}
// Cacheable files are files that should never change. Typically
// named by their hash (eg meteor bundled js and css files).
// We cache them ~forever (1yr).
@@ -302,48 +278,38 @@ var runWebAppServer = function () {
// to include a query parameter to bust caches. That way we can both get
// good caching behavior and allow users to change assets without delay.
// https://github.com/meteor/meteor/issues/773
//
// Because a source map's URL is constructed from the URL of the file it is
// the source map of, we assume that a source map is cacheable iff the file
// is cacheable.
var maxAge = info.cacheable
? 1000 * 60 * 60 * 24 * 365
: 1000 * 60 * 60 * 24;
// Tell the client where to find the source map for this file.
if (urlToSourceMap) {
// This should just be SourceMap, but slightly more versions of Chrome
// support the older X-SourceMap.
//
// To figure out if your version of Chrome should support SourceMap,
// - go to chrome://version. Let's say the Chrome version is
// 28.0.1500.71 and the Blink version is 537.36 (@153022)
// - go to http://src.chromium.org/viewvc/blink/branches/chromium/1500/Source/core/inspector/InspectorPageAgent.cpp?view=log
// where the "1500" is the third part of your Chrome version
// - find the first revision that is no greater than the "153022"
// number. That's probably the first one and it probably has
// a message of the form "Branch 1500 - blink@r149738"
// - If *that* revision number (149738) is at least 151755,
// then Chrome should support SourceMap (not just X-SourceMap)
// (The change is https://codereview.chromium.org/15832007)
//
// You also need to enable source maps in Chrome: open dev tools, click
// the gear in the bottom right corner, and select "enable source maps".
//
// Firefox 23+ supports source maps (and they are on by default in 24+),
// but doesn't support either header yet:
// https://bugzilla.mozilla.org/show_bug.cgi?id=765993
// We could make FF work by adding a comment to the end of the source map
// file. But that would require doing one of the following:
// - determining the source map URL at bundle time instead of here
// - writing to the source directory
// - not using the send module (or hacking it to allow a footer)
// None of these alternatives are great, so for now we just hope that
// FF will implement one of the headers soon.
res.setHeader('X-SourceMap', urlToSourceMap);
}
// It sure would be nice to write this here:
// res.setHeader('X-SourceMap', urlToSourceMap);
// Or even just SourceMap, but slightly more versions of Chrome support the
// older X-SourceMap.
//
// But we'd like to support FF, so for now we just use the "//@" comment at
// the end of the source map file.
//
// To figure out if your version of Chrome should support SourceMap,
// - go to chrome://version. Let's say the Chrome version is
// 28.0.1500.71 and the Blink version is 537.36 (@153022)
// - go to http://src.chromium.org/viewvc/blink/branches/chromium/1500/Source/core/inspector/InspectorPageAgent.cpp?view=log
// where the "1500" is the third part of your Chrome version
// - find the first revision that is no greater than the "153022"
// number. That's probably the first one and it probably has
// a message of the form "Branch 1500 - blink@r149738"
// - If *that* revision number (149738) is at least 151755,
// then Chrome should support SourceMap (not just X-SourceMap)
// (The change is https://codereview.chromium.org/15832007)
//
// You also need to enable source maps in Chrome: open dev tools, click
// the gear in the bottom right corner, and select "enable source maps".
//
// Firefox 23+ supports source maps (and they are on by default in 24+),
// but doesn't support either header yet:
// https://bugzilla.mozilla.org/show_bug.cgi?id=765993
send(req, path.join(clientDir, fileToServe))
send(req, path.join(clientDir, info.path))
.maxage(maxAge)
.hidden(true) // if we specified a dotfile in the manifest, serve it
.on('error', function (err) {
@@ -352,7 +318,7 @@ var runWebAppServer = function () {
res.end();
})
.on('directory', function () {
Log.error("Unexpected directory " + fileToServe);
Log.error("Unexpected directory " + info.path);
res.writeHead(500);
res.end();
})

View File

@@ -303,6 +303,15 @@ _.extend(File.prototype, {
return encoding ? self._contents.toString(encoding) : self._contents;
},
setContents: function (b) {
var self = this;
if (!(b instanceof Buffer))
throw new Error("Must set contents to a Buffer");
self._contents = b;
// Un-cache hash.
self._hash = null;
},
size: function () {
var self = this;
return self.contents().length;
@@ -863,16 +872,14 @@ _.extend(ClientTarget.prototype, {
// Build up a manifest of all resources served via HTTP.
var manifest = [];
eachResource(function (file, type) {
writeFile(file, builder);
var fileContents = file.contents();
var manifestItem = {
path: file.targetPath,
where: "client",
type: type,
cacheable: file.cacheable,
url: file.url,
size: file.size(),
hash: file.hash()
url: file.url
};
if (file.sourceMap) {
@@ -886,8 +893,26 @@ _.extend(ClientTarget.prototype, {
var mapData = new Buffer(")]}'\n" + file.sourceMap, 'utf8');
manifestItem.sourceMap = builder.writeToGeneratedFilename(
file.targetPath + '.map', {data: mapData});
// Use a SHA to make this cacheable.
var sourceMapBaseName = file.hash() + ".map";
// XXX When we can, drop all of this and just use the SourceMap
// header. FF doesn't support that yet, though.
// https://bugzilla.mozilla.org/show_bug.cgi?id=765993
file.setContents(Buffer.concat([
file.contents(),
new Buffer("\n//@ sourceMappingURL=" + sourceMapBaseName + "\n")
]));
manifestItem.sourceMapUrl = require('url').resolve(
file.url, sourceMapBaseName);
}
// Set this now, in case we mutated the file's contents.
manifestItem.size = file.size();
manifestItem.hash = file.hash();
writeFile(file, builder);
manifest.push(manifestItem);
});

View File

@@ -119,10 +119,17 @@ _.extend(Module.prototype, {
file: file.servePath
}); // results has 'code' and 'map' attributes
var sourceMap = results.map.toJSON();
// No use generating empty source maps.
if (_.isEmpty(sourceMap.sources))
sourceMap = null;
else
sourceMap = JSON.stringify(sourceMap);
return {
source: results.code,
servePath: file.servePath,
sourceMap: results.map.toString()
sourceMap: sourceMap
};
});
}
@@ -313,9 +320,9 @@ _.extend(File.prototype, {
var self = this;
if (self.module.name)
return "packages/" + self.module.name + "/" + self.sourcePath;
return self.module.name + "/" + self.sourcePath;
else
return "app/" + self.sourcePath;
return require('path').basename(self.sourcePath);
},
runLinkerFileTransform: function (exports) {
@@ -346,10 +353,13 @@ _.extend(File.prototype, {
mapNode = sourcemap.SourceNode.fromStringWithSourceMap(
self.source, new sourcemap.SourceMapConsumer(self.sourceMap));
} else {
mapNode = new sourcemap.SourceNode(1, 0, self._pathForSourceMap(),
self.source);
// Provide the original content as well.
mapNode.setSourceContent(self._pathForSourceMap(), self.source);
// This is an app file that was always JS. The output file here is going
// to be the same name as the input file (because _pathForSourceMap in
// apps is the basename of the source file), and having a JS file
// pointing to a source map pointing to a JS file of the same name will
// (a) be confusing (b) be unnecessary since we aren't renumbering
// anything and (c) confuse at least Chrome.
mapNode = self.source;
}
return new sourcemap.SourceNode(null, null, null, [

View File

@@ -361,8 +361,8 @@ _.extend(Slice.prototype, {
// XXX duplicates _pathForSourceMap() in linker
pathForSourceMap: (
self.pkg.name
? "packages/" + self.pkg.name + "/" + relPath
: "app/" + relPath),
? self.pkg.name + "/" + relPath
: path.basename(relPath)),
// null if this is an app. intended to be used for the sources
// dictionary for source maps.
packageName: self.pkg.name,