Permalink
Browse files

metro-bundler: use buildID instead of entry path for reporting

Summary: This fixes a longstanding bug that happens when 2 bundles with the same entry path but different options (dev, minify, etc.) get mixed up in the reporting. To prevent that we just use a unique build ID for each bundle that the Server handles separately.

Reviewed By: cpojer

Differential Revision: D5147049

fbshipit-source-id: da5c9cfe8c6a5d888b5be737947800d213081d86
  • Loading branch information...
jeanlauliac authored and facebook-github-bot committed May 31, 2017
1 parent c2eb9f4 commit b74b2d5193d29ac364852a02df752eb8dd5ec177
Showing with 49 additions and 30 deletions.
  1. +25 −8 packager/src/Server/index.js
  2. +20 −19 packager/src/lib/TerminalReporter.js
  3. +4 −3 packager/src/lib/reporting.js
@@ -154,6 +154,7 @@ class Server {
_reporter: Reporter;
_symbolicateInWorker: Symbolicate;
_platforms: Set<string>;
_nextBundleBuildID: number;
constructor(options: Options) {
this._opts = {
@@ -239,6 +240,7 @@ class Server {
}, 50);
this._symbolicateInWorker = symbolicate.createWorker();
this._nextBundleBuildID = 1;
}
end(): mixed {
@@ -491,29 +493,34 @@ class Server {
* and for when we build for scratch.
*/
_reportBundlePromise(
buildID: string,
options: {entryFile: string},
bundlePromise: Promise<Bundle>,
): Promise<Bundle> {
this._reporter.update({
buildID,
entryFilePath: options.entryFile,
type: 'bundle_build_started',
});
return bundlePromise.then(bundle => {
this._reporter.update({
entryFilePath: options.entryFile,
buildID,
type: 'bundle_build_done',
});
return bundle;
}, error => {
this._reporter.update({
entryFilePath: options.entryFile,
buildID,
type: 'bundle_build_failed',
});
return Promise.reject(error);
});
}
useCachedOrUpdateOrCreateBundle(options: BundleOptions): Promise<Bundle> {
useCachedOrUpdateOrCreateBundle(
buildID: string,
options: BundleOptions,
): Promise<Bundle> {
const optionsJson = this.optionsHash(options);
const bundleFromScratch = () => {
const building = this.buildBundle(options);
@@ -606,15 +613,15 @@ class Server {
debug('Failed to update existing bundle, rebuilding...', e.stack || e.message);
return bundleFromScratch();
});
return this._reportBundlePromise(options, bundlePromise);
return this._reportBundlePromise(buildID, options, bundlePromise);
} else {
debug('Using cached bundle');
return bundle;
}
});
}
return this._reportBundlePromise(options, bundleFromScratch());
return this._reportBundlePromise(buildID, options, bundleFromScratch());
}
processRequest(
@@ -660,12 +667,13 @@ class Server {
entry_point: options.entryFile,
}));
const buildID = this.getNewBuildID();
let reportProgress = emptyFunction;
if (!this._opts.silent) {
reportProgress = (transformedFileCount, totalFileCount) => {
this._reporter.update({
buildID,
type: 'bundle_transform_progressed',
entryFilePath: options.entryFile,
transformedFileCount,
totalFileCount,
});
@@ -679,7 +687,7 @@ class Server {
};
debug('Getting bundle for request');
const building = this.useCachedOrUpdateOrCreateBundle(options);
const building = this.useCachedOrUpdateOrCreateBundle(buildID, options);
building.then(
p => {
if (requestType === 'bundle') {
@@ -780,7 +788,12 @@ class Server {
_sourceMapForURL(reqUrl: string): Promise<SourceMap> {
const options = this._getOptionsFromUrl(reqUrl);
const building = this.useCachedOrUpdateOrCreateBundle(options);
// We're not properly reporting progress here. Reporting should be done
// from within that function.
const building = this.useCachedOrUpdateOrCreateBundle(
this.getNewBuildID(),
options,
);
return building.then(p => p.getSourceMap({
minify: options.minify,
dev: options.dev,
@@ -898,6 +911,10 @@ class Server {
return query[opt] === 'true' || query[opt] === '1';
}
getNewBuildID(): string {
return (this._nextBundleBuildID++).toString(36);
}
static DEFAULT_BUNDLE_OPTIONS;
}
@@ -26,6 +26,7 @@ const GLOBAL_CACHE_DISABLED_MESSAGE_FORMAT =
'The global cache is now disabled because %s';
type BundleProgress = {
entryFilePath: string,
transformedFileCount: number,
totalFileCount: number,
ratio: number,
@@ -43,8 +44,8 @@ function getProgressBar(ratio: number, length: number) {
}
export type TerminalReportableEvent = ReportableEvent | {
buildID: string,
type: 'bundle_transform_progressed_throttled',
entryFilePath: string,
transformedFileCount: number,
totalFileCount: number,
};
@@ -66,7 +67,7 @@ class TerminalReporter {
_dependencyGraphHasLoaded: boolean;
_scheduleUpdateBundleProgress: (data: {
entryFilePath: string,
buildID: string,
transformedFileCount: number,
totalFileCount: number,
}) => void;
@@ -86,8 +87,7 @@ class TerminalReporter {
* Bunding `foo.js` |#### | 34.2% (324/945)
*/
_getBundleStatusMessage(
entryFilePath: string,
{totalFileCount, transformedFileCount, ratio}: BundleProgress,
{entryFilePath, totalFileCount, transformedFileCount, ratio}: BundleProgress,
phase: BuildPhase,
): string {
const localPath = path.relative('.', entryFilePath);
@@ -114,10 +114,10 @@ class TerminalReporter {
}
}
_logBundleBuildDone(entryFilePath: string) {
const progress = this._activeBundles.get(entryFilePath);
_logBundleBuildDone(buildID: string) {
const progress = this._activeBundles.get(buildID);
if (progress != null) {
const msg = this._getBundleStatusMessage(entryFilePath, {
const msg = this._getBundleStatusMessage({
...progress,
ratio: 1,
transformedFileCount: progress.totalFileCount,
@@ -126,10 +126,10 @@ class TerminalReporter {
}
}
_logBundleBuildFailed(entryFilePath: string) {
const progress = this._activeBundles.get(entryFilePath);
_logBundleBuildFailed(buildID: string) {
const progress = this._activeBundles.get(buildID);
if (progress != null) {
const msg = this._getBundleStatusMessage(entryFilePath, progress, 'failed');
const msg = this._getBundleStatusMessage(progress, 'failed');
terminal.log(msg);
}
}
@@ -197,10 +197,10 @@ class TerminalReporter {
this._logPackagerInitializingFailed(event.port, event.error);
break;
case 'bundle_build_done':
this._logBundleBuildDone(event.entryFilePath);
this._logBundleBuildDone(event.buildID);
break;
case 'bundle_build_failed':
this._logBundleBuildFailed(event.entryFilePath);
this._logBundleBuildFailed(event.buildID);
break;
case 'bundling_error':
this._logBundlingError(event.error);
@@ -249,13 +249,13 @@ class TerminalReporter {
* also prevent the ratio from going backwards.
*/
_updateBundleProgress(
{entryFilePath, transformedFileCount, totalFileCount}: {
entryFilePath: string,
{buildID, transformedFileCount, totalFileCount}: {
buildID: string,
transformedFileCount: number,
totalFileCount: number,
},
) {
const currentProgress = this._activeBundles.get(entryFilePath);
const currentProgress = this._activeBundles.get(buildID);
if (currentProgress == null) {
return;
}
@@ -277,10 +277,11 @@ class TerminalReporter {
switch (event.type) {
case 'bundle_build_done':
case 'bundle_build_failed':
this._activeBundles.delete(event.entryFilePath);
this._activeBundles.delete(event.buildID);
break;
case 'bundle_build_started':
this._activeBundles.set(event.entryFilePath, {
this._activeBundles.set(event.buildID, {
entryFilePath: event.entryFilePath,
transformedFileCount: 0,
totalFileCount: 1,
ratio: 0,
@@ -322,8 +323,8 @@ class TerminalReporter {
return [
this._getDepGraphStatusMessage(),
].concat(Array.from(this._activeBundles.entries()).map(
([entryFilePath, progress]) =>
this._getBundleStatusMessage(entryFilePath, progress, 'in_progress'),
([_, progress]) =>
this._getBundleStatusMessage(progress, 'in_progress'),
)).filter(str => str != null).join('\n');
}
@@ -33,12 +33,13 @@ export type ReportableEvent = {
port: number,
error: Error,
} | {
entryFilePath: string,
buildID: string,
type: 'bundle_build_done',
} | {
entryFilePath: string,
buildID: string,
type: 'bundle_build_failed',
} | {
buildID: string,
entryFilePath: string,
type: 'bundle_build_started',
} | {
@@ -49,8 +50,8 @@ export type ReportableEvent = {
} | {
type: 'dep_graph_loaded',
} | {
buildID: string,
type: 'bundle_transform_progressed',
entryFilePath: string,
transformedFileCount: number,
totalFileCount: number,
} | {

0 comments on commit b74b2d5

Please sign in to comment.