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.

Sign up for GitHub

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

Jump to bottom

fix: do not re-generate source unnecessarily #321

Merged
merged 2 commits into from
Jun 20, 2019
Merged

fix: do not re-generate source unnecessarily #321

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ebd15f1
fix(android): avoid running generate on AST if we haven't modified it
sgtcoolguy Jun 20, 2019
ad081f1
fix(ios): avoid running generate on AST if we have't modified it
sgtcoolguy Jun 20, 2019
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
9 changes: 8 additions & 1 deletion android/hooks/tasks/scan-references-task.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@ class ScanReferencesTask extends IncrementalFileTask {
this._logger.trace('Searching for hyperloop requires in: ' + file);
const logger = this.logger;
const self = this;
let changedAST = false;
const HyperloopVisitor = {
// ES5-style require calls
CallExpression: function(p) {
Expand All @@ -264,6 +265,7 @@ class ScanReferencesTask extends IncrementalFileTask {
p.replaceWith(
t.callExpression(p.node.callee, [t.stringLiteral('hyperloop/' + packageName)])
);
changedAST = true;
}
} else {
// single type
Expand All @@ -274,6 +276,7 @@ class ScanReferencesTask extends IncrementalFileTask {
p.replaceWith(
t.callExpression(p.node.callee, [t.stringLiteral('hyperloop/' + validatedClassName)])
);
changedAST = true;
}
}
}
Expand Down Expand Up @@ -302,6 +305,7 @@ class ScanReferencesTask extends IncrementalFileTask {
p.replaceWith(
t.importDeclaration(p.node.specifiers, t.stringLiteral('hyperloop/' + packageName))
);
changedAST = true;
}
} else {
// single type
Expand All @@ -314,6 +318,7 @@ class ScanReferencesTask extends IncrementalFileTask {
p.replaceWith(
t.importDeclaration(p.node.specifiers, t.stringLiteral('hyperloop/' + validatedClassName))
);
changedAST = true;
}
}
}
Expand All @@ -323,7 +328,9 @@ class ScanReferencesTask extends IncrementalFileTask {
// Now traverse the AST and generate modified source
const ast = babelParser.parse(originalSource, { sourceFilename: file, sourceType: 'unambiguous' });
traverse(ast, HyperloopVisitor);
const modifiedSource = generate(ast, {}).code;
// if we didn't change the AST, no need to generate new source!
// If we *do* generate new source, try to retain the lines and comments to retain source map
const modifiedSource = changedAST ? generate(ast, { retainLines: true, comments: true }, originalSource).code : originalSource;

return {
usedClasses: usedClasses,
Expand Down
16 changes: 11 additions & 5 deletions iphone/hooks/hyperloop.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,7 @@ HyperloopiOSBuilder.prototype.patchJSFile = function patchJSFile(obj, sourceFile
// require() calls with the Hyperloop layer
const requireRegexp = /[\w_/\-\\.]+/ig;
const self = this;
let changedAST = false;
const HyperloopVisitor = {
// ES5-style require calls
CallExpression: function (p) {
Expand Down Expand Up @@ -505,6 +506,7 @@ HyperloopiOSBuilder.prototype.patchJSFile = function patchJSFile(obj, sourceFile
p.replaceWith(
t.callExpression(p.node.callee, [ t.stringLiteral('/' + ref) ])
);
changedAST = true;
}
},
// ES6+-style imports
Expand Down Expand Up @@ -587,19 +589,23 @@ HyperloopiOSBuilder.prototype.patchJSFile = function patchJSFile(obj, sourceFile
});

// Apply replacements
if (replacements.length == 1) {
const replaceCount = replacements.length;
if (replaceCount === 1) {
p.replaceWith(replacements[0]);
} else {
//
changedAST = true;
} else if (replaceCount > 1) {
p.replaceWithMultiple(replacements);
changedAST = true;
}
}
}
};

const ast = babelParser.parse(contents, { sourceFilename: sourceFilename, sourceType: 'unambiguous' });
traverse(ast, HyperloopVisitor);
let newContents = generate(ast, {}).code;
// if we didn't change the AST, no need to generate new source!
// If we *do* generate new source, try to retain the lines and comments to retain source map
let newContents = changedAST ? generate(ast, { retainLines: true, comments: true }, contents).code : contents;

// TODO: Remove once we combine the custom acorn-based parser and the babelParser parser above!
// Or maybe it can go now? The migration stuff is noted that it could be removed in 3.0.0...
Expand All @@ -616,7 +622,7 @@ HyperloopiOSBuilder.prototype.patchJSFile = function patchJSFile(obj, sourceFile
this.logger.debug('No change, skipping ' + chalk.cyan(destinationFilename));
} else {
this.logger.debug('Writing ' + chalk.cyan(destinationFilename));
// modify the contents stored int he state object passed thorugh the hook,
// modify the contents stored in the state object passed through the hook,
// so that SDK CLI can use new contents for minification/transpilation
obj.contents = newContents;
}
Expand Down