Skip to content

Commit

Permalink
Add option to bundle server to generate full sourcemaps from babel
Browse files Browse the repository at this point in the history 
Reviewed By: bestander

Differential Revision: D3863894

fbshipit-source-id: a282758e022d403743841bc59277196e6741ed18
  • Loading branch information
@cwdick
cwdick authored and Facebook Github Bot 6 committed Sep 15, 2016
1 parent fa6191f commit e6bec9c
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 5 deletions.
3 changes: 2 additions & 1 deletion local-cli/server/middleware/heapCaptureMiddleware.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,11 @@ function getSourceMapForUrl(url, onFailure, onSuccess) {
}

const parsedUrl = urlLib.parse(url);
const mapPath = parsedUrl.pathname.replace(/\.bundle$/, '.map');
const options = {
host: 'localhost',
port: parsedUrl.port,
path: parsedUrl.pathname.replace(/\.bundle$/, '.map') + parsedUrl.search,
path: mapPath + parsedUrl.search + '&babelSourcemap=true',
};

http.get(options, (res) => {
Expand Down
3 changes: 2 additions & 1 deletion local-cli/server/middleware/jscProfilerMiddleware.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,11 @@ class TreeTransformator {
}

const parsedUrl = urlLib.parse(url);
const mapPath = parsedUrl.pathname.replace(/\.bundle$/, '.map');
const options = {
host: 'localhost',
port: parsedUrl.port,
path: parsedUrl.pathname.replace(/\.bundle$/, '.map') + parsedUrl.search,
path: mapPath + parsedUrl.search + '&babelSourcemap=true',
};

http.get(options, (res) => {
Expand Down
5 changes: 4 additions & 1 deletion packager/react-packager/src/Bundler/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,7 @@ class Bundler {
entryModuleOnly,
resolutionResponse,
isolateModuleIDs,
generateSourceMaps,
}) {
const onResolutionResponse = response => {
bundle.setMainModuleId(response.getModuleId(getMainModule(response)));
Expand Down Expand Up @@ -301,6 +302,7 @@ class Bundler {
onResolutionResponse,
finalizeBundle,
isolateModuleIDs,
generateSourceMaps,
});
}

Expand Down Expand Up @@ -352,6 +354,7 @@ class Bundler {
unbundle,
resolutionResponse,
isolateModuleIDs,
generateSourceMaps,
onResolutionResponse = noop,
onModuleTransformed = noop,
finalizeBundle = noop,
Expand Down Expand Up @@ -385,7 +388,7 @@ class Bundler {
onProgress,
minify,
isolateModuleIDs,
generateSourceMaps: unbundle,
generateSourceMaps: unbundle || generateSourceMaps,
});
}

Expand Down
9 changes: 7 additions & 2 deletions packager/react-packager/src/Server/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,10 @@ const bundleOpts = declareOpts({
resolutionResponse: {
type: 'object',
},
generateSourceMaps: {
type: 'boolean',
required: false,
}
});

const dependencyOpts = declareOpts({
Expand Down Expand Up @@ -814,7 +818,7 @@ class Server {
sourceMapUrl: url.format(sourceMapUrlObj),
entryFile: entryFile,
dev: this._getBoolOptionFromQuery(urlObj.query, 'dev', true),
minify: this._getBoolOptionFromQuery(urlObj.query, 'minify'),
minify: this._getBoolOptionFromQuery(urlObj.query, 'minify', false),
hot: this._getBoolOptionFromQuery(urlObj.query, 'hot', false),
runModule: this._getBoolOptionFromQuery(urlObj.query, 'runModule', true),
inlineSourceMap: this._getBoolOptionFromQuery(
Expand All @@ -828,11 +832,12 @@ class Server {
'entryModuleOnly',
false,
),
generateSourceMaps: this._getBoolOptionFromQuery(urlObj.query, 'babelSourcemap'),
};
}

_getBoolOptionFromQuery(query, opt, defaultVal) {
if (query[opt] == null && defaultVal != null) {
if (query[opt] == null) {
return defaultVal;
}

Expand Down

7 comments on commit e6bec9c

@Kerumen
Copy link
Contributor

@Kerumen Kerumen commented on e6bec9c Oct 2, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

to generate full sourcemaps

What does this means? Before it was not the full sourcemaps?

@cwdick
Copy link
Contributor Author

@cwdick cwdick commented on e6bec9c Oct 3, 2016

Choose a reason for hiding this comment

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

This forces using babel sourcemaps that have full column information for mapped files. This is needed for things the resolving error and CPU profile stacks (but not debugging, which is only line based). Without this flag, sourcemaps are generated by _getMappings or generateSourceMapForVirtualModule in Bundle.js, which work, but don't have sub-line granularity.

It's a flag and not on all the time because it makes the sourcemaps way bigger, so it's slower to build the bundle, and can cause chrome dev tools to crash (out of memory!) when debugging.

@jondot
Copy link
Contributor

@jondot jondot commented on e6bec9c Oct 13, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

@cwdick could you define sub-line granularity? we're trying to figure out if this is why we never got exact mapping position in crashlytics, and now I'm seeing the same fuzziness in Sentry (all in production builds). So I'm wondering if after this patch we'll be good to go.

@Kerumen
Copy link
Contributor

Choose a reason for hiding this comment

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

@jondot Did you achieve to get Sentry working with React Native ? I uploaded the bundle and the sourcemaps and I always get error like Source code was not found for /Users/[path]/node_modules/react/lib/ReactNativeBaseComponent.js and so on for all the components in the stacktrace.

@jondot
Copy link
Contributor

@jondot jondot commented on e6bec9c Oct 13, 2016

Choose a reason for hiding this comment

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

@Kerumen yes, I did. This is what I do (may be different from your workflow):

  1. run bundle with react-native bundle with production flags (dev=false), and per platform (ios/android), and include sourcemaps
  2. create a release per platform, and upload the bundles with the maps, but name them very specifically with a leading slash this is where you might went wrong, so for main.jsbundle you must name the release asset /main.jsbundle.

@jondot
Copy link
Contributor

@jondot jondot commented on e6bec9c Oct 14, 2016

Choose a reason for hiding this comment

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

@Kerumen sure

@cwdick
Copy link
Contributor Author

@cwdick cwdick commented on e6bec9c Oct 14, 2016

Choose a reason for hiding this comment

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

@jondot, I don't think this would do anything for that, you're still just looking at line numbers. The extra info in the sourcemaps is for mapping columns too.

Please sign in to comment.