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

build(broccoli): store DiffResult for re-use only if DiffResult #2699

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
54 changes: 40 additions & 14 deletions tools/broccoli/diffing-broccoli-plugin.ts
Expand Up @@ -28,7 +28,7 @@ export function wrapDiffingPlugin(pluginClass): DiffingPluginWrapperFactory {


export interface DiffingBroccoliPlugin {
rebuild(diff: (DiffResult | DiffResult[])): (Promise<any>| void);
rebuild(diff: (DiffResult | DiffResult[])): (Promise<DiffResult | void>| DiffResult | void);
cleanup ? () : void;
}

Expand All @@ -52,6 +52,8 @@ class DiffingPluginWrapper implements BroccoliTree {
cachePath = null;
outputPath = null;

private diffResult: DiffResult = null;

constructor(private pluginClass, private wrappedPluginArguments) {
if (Array.isArray(wrappedPluginArguments[0])) {
this.inputTrees = this.stabilizeTrees(wrappedPluginArguments[0]);
Expand All @@ -62,33 +64,57 @@ class DiffingPluginWrapper implements BroccoliTree {
this.description = this.pluginClass.name;
}

private calculateDiff(firstRun: boolean): (DiffResult | DiffResult[]) {
// TODO(caitp): optionally log trees based on environment variable or
// command line option. It may be worth logging for trees where elapsed
// time exceeds some threshold, like 10ms.
if (this.treeDiffer) {
return this.treeDiffer.diffTree();
} else if (this.treeDiffers) {
return this.treeDiffers.map((treeDiffer) => treeDiffer.diffTree());
private getDiffResult(): (DiffResult | DiffResult[]) {
let returnOrCalculateDiffResult = (tree, index) => {
// returnOrCalculateDiffResult will do one of two things:
//
// If `this.diffResult` is null, calculate a DiffResult using TreeDiffer
// for the input tree.
//
// Otherwise, `this.diffResult` was produced from the output of the
// inputTree's rebuild() method, and can be used without being checked.
// Set `this.diffResult` to null and return the previously stored value.
let diffResult = tree.diffResult;
if (diffResult) return diffResult;
let differ = index === false ? this.treeDiffer : this.treeDiffers[index];
return differ.diffTree();
};

if (this.inputTrees) {
return this.inputTrees.map(returnOrCalculateDiffResult);
} else if (this.inputTree) {
return returnOrCalculateDiffResult(this.inputTree, false);
} else {
throw new Error("Missing TreeDiffer");
}
}

private maybeStoreDiffResult(value: (DiffResult | void)) {
if (!(value instanceof DiffResult)) value = null;
this.diffResult = <DiffResult>(value);
}

rebuild() {
rebuild(): (Promise<any>| void) {
try {
let firstRun = !this.initialized;
this.init();

let diffResult = this.calculateDiff(firstRun);
let diffResult = this.getDiffResult();

var rebuildPromise = this.wrappedPlugin.rebuild(diffResult);
let result = this.wrappedPlugin.rebuild(diffResult);

if (rebuildPromise) {
return (<Promise<any>>rebuildPromise).then(this.relinkOutputAndCachePaths.bind(this));
if (result) {
let resultPromise = <Promise<DiffResult | void>>(result);
if (resultPromise.then) {
// rebuild() -> Promise<>
return resultPromise.then((result: (DiffResult | void)) => {
this.maybeStoreDiffResult(result);
this.relinkOutputAndCachePaths();
});
}
}

this.maybeStoreDiffResult(<(DiffResult | void)>(result));
this.relinkOutputAndCachePaths();
} catch (e) {
e.message = `[${this.description}]: ${e.message}`;
Expand Down
27 changes: 15 additions & 12 deletions tools/broccoli/tree-differ.ts
Expand Up @@ -121,25 +121,28 @@ export class TreeDiffer {
}


export interface DiffResult {
addedPaths: string[];
changedPaths: string[];
removedPaths: string[];
log(verbose: boolean): void;
toString(): string;
}
export class DiffResult {
public addedPaths: string[] = [];
public changedPaths: string[] = [];
public removedPaths: string[] = [];

constructor(public label: string = '') {}

log(verbose: boolean): void {}

class DirtyCheckingDiffResult {
toString(): string {
// TODO(@caitp): more meaningful logging
return '';
}
}

class DirtyCheckingDiffResult extends DiffResult {
public filesChecked: number = 0;
public directoriesChecked: number = 0;
public addedPaths: string[] = [];
public changedPaths: string[] = [];
public removedPaths: string[] = [];
public startTime: number = Date.now();
public endTime: number = null;

constructor(public label: string, public directoryName: string) {}
constructor(label: string, public directoryName: string) { super(label); }

toString() {
return `${pad(this.label, 30)}, ${pad(this.endTime - this.startTime, 5)}ms, ` +
Expand Down