Skip to content
This repository was archived by the owner on Apr 4, 2025. It is now read-only.

Build optimizer es2015 #169

Merged
merged 3 commits into from
Sep 26, 2017
Merged

Build optimizer es2015 #169

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
edd0d27
ci: accept revert commits
filipesilva Sep 26, 2017
c94f630
revert: "fix(@angular-devkit/build-optimizer): don't add pure to supe…
filipesilva Sep 26, 2017
22fa623
feat(@angular-devkit/build-optimizer): support es2015 modules
filipesilva Sep 26, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 25 additions & 15 deletions packages/angular_devkit/build_optimizer/src/build-optimizer/build-optimizer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,29 @@ import { getWrapEnumsTransformer, testWrapEnums } from '../transforms/wrap-enums


const whitelistedAngularModules = [
/(\\|\/)node_modules(\\|\/)@angular(\\|\/)animations(\\|\/)/,
/(\\|\/)node_modules(\\|\/)@angular(\\|\/)common(\\|\/)/,
/(\\|\/)node_modules(\\|\/)@angular(\\|\/)compiler(\\|\/)/,
/(\\|\/)node_modules(\\|\/)@angular(\\|\/)core(\\|\/)/,
/(\\|\/)node_modules(\\|\/)@angular(\\|\/)forms(\\|\/)/,
/(\\|\/)node_modules(\\|\/)@angular(\\|\/)http(\\|\/)/,
/(\\|\/)node_modules(\\|\/)@angular(\\|\/)platform-browser-dynamic(\\|\/)/,
/(\\|\/)node_modules(\\|\/)@angular(\\|\/)platform-browser(\\|\/)/,
/(\\|\/)node_modules(\\|\/)@angular(\\|\/)platform-webworker-dynamic(\\|\/)/,
/(\\|\/)node_modules(\\|\/)@angular(\\|\/)platform-webworker(\\|\/)/,
/(\\|\/)node_modules(\\|\/)@angular(\\|\/)router(\\|\/)/,
/(\\|\/)node_modules(\\|\/)@angular(\\|\/)upgrade(\\|\/)/,
/(\\|\/)node_modules(\\|\/)@angular(\\|\/)material(\\|\/)/,
/(\\|\/)node_modules(\\|\/)@angular(\\|\/)cdk(\\|\/)/,
/[\\\/]node_modules[\\\/]@angular[\\\/]animations[\\\/]/,
/[\\\/]node_modules[\\\/]@angular[\\\/]common[\\\/]/,
/[\\\/]node_modules[\\\/]@angular[\\\/]compiler[\\\/]/,
/[\\\/]node_modules[\\\/]@angular[\\\/]core[\\\/]/,
/[\\\/]node_modules[\\\/]@angular[\\\/]forms[\\\/]/,
/[\\\/]node_modules[\\\/]@angular[\\\/]http[\\\/]/,
/[\\\/]node_modules[\\\/]@angular[\\\/]platform-browser-dynamic[\\\/]/,
/[\\\/]node_modules[\\\/]@angular[\\\/]platform-browser[\\\/]/,
/[\\\/]node_modules[\\\/]@angular[\\\/]platform-webworker-dynamic[\\\/]/,
/[\\\/]node_modules[\\\/]@angular[\\\/]platform-webworker[\\\/]/,
/[\\\/]node_modules[\\\/]@angular[\\\/]router[\\\/]/,
/[\\\/]node_modules[\\\/]@angular[\\\/]upgrade[\\\/]/,
/[\\\/]node_modules[\\\/]@angular[\\\/]material[\\\/]/,
/[\\\/]node_modules[\\\/]@angular[\\\/]cdk[\\\/]/,
];

const es5AngularModules = [
// Angular 4 packaging format has .es5.js as the extension.
/.es5.js$/, // Angular 4
// Angular 5 has esm5 folders.
/[\\\/]node_modules[\\\/]@angular[\\\/][^\\/][\\\/]esm5[\\\/]/,
// All Angular versions have UMD with es5.
/.umd.js$/,
];

export interface BuildOptimizerOptions {
Expand Down Expand Up @@ -70,9 +79,10 @@ export function buildOptimizer(options: BuildOptimizerOptions): TransformJavascr

if (inputFilePath
&& whitelistedAngularModules.some((re) => re.test(inputFilePath))
&& es5AngularModules.some((re) => re.test(inputFilePath))
) {
getTransforms.push(
// getPrefixFunctionsTransformer is rather dangerous, apply only to known pure modules.
// getPrefixFunctionsTransformer is rather dangerous, apply only to known pure es5 modules.
// It will mark both `require()` calls and `console.log(stuff)` as pure.
// We only apply it to whitelisted modules, since we know they are safe.
// getPrefixFunctionsTransformer needs to be before getFoldFileTransformer.
Expand Down
18 changes: 18 additions & 0 deletions packages/angular_devkit/build_optimizer/src/build-optimizer/build-optimizer_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,24 @@ describe('build-optimizer', () => {
expect(boOutput.content).toBeFalsy();
expect(boOutput.emitSkipped).toEqual(true);
});

it('supports es2015 modules', () => {
// prefix-functions would add PURE_IMPORTS_START and PURE to the super call.
// This test ensures it isn't applied to es2015 modules.
const output = oneLine`
import { Injectable } from '@angular/core';
class Clazz extends BaseClazz { constructor(e) { super(e); } }
`;
const input = stripIndent`
${output}
Clazz.ctorParameters = () => [ { type: Injectable } ];
`;

const inputFilePath = '/node_modules/@angular/core/@angular/core.js';
const boOutput = buildOptimizer({ content: input, inputFilePath });
expect(oneLine`${boOutput.content}`).toEqual(output);
expect(boOutput.emitSkipped).toEqual(false);
});
});


Expand Down
7 changes: 0 additions & 7 deletions packages/angular_devkit/build_optimizer/src/transforms/prefix-functions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ export function findTopLevelFunctions(parentNode: ts.Node): ts.Node[] {
topLevelFunctions.push(node.parent);
} else if ((node.kind === ts.SyntaxKind.CallExpression
|| node.kind === ts.SyntaxKind.NewExpression)
&& !isSuperCall(node)
&& !hasPureComment(node)
) {
topLevelFunctions.push(node);
Expand Down Expand Up @@ -118,9 +117,3 @@ function hasPureComment(node: ts.Node) {

return leadingComment && leadingComment.some((comment) => comment.text === pureFunctionComment);
}

function isSuperCall(node: ts.Node) {
const callExpr = node as ts.CallExpression;

return callExpr.expression && callExpr.expression.kind === ts.SyntaxKind.SuperKeyword;
}
18 changes: 1 addition & 17 deletions packages/angular_devkit/build_optimizer/src/transforms/prefix-functions_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ describe('prefix-functions', () => {
expect(oneLine`${transform(input)}`).toEqual(oneLine`${output}`);
});

it('doesn\'t add comment when inside function declarations or expressions', () => {
it('doesn\'t adds comment when inside function declarations or expressions', () => {
const input = stripIndent`
function funcDecl() {
var newClazz = Clazz();
Expand All @@ -102,21 +102,5 @@ describe('prefix-functions', () => {

expect(oneLine`${transform(input)}`).toEqual(oneLine`${output}`);
});

it('doesn\'t add comment to super calls', () => {
const input = oneLine`
class ExtendedClass extends BaseClass {
constructor(e) {
super(e);
}
}
`;
const output = stripIndent`
${emptyImportsComment}
${input}
`;

expect(oneLine`${transform(input)}`).toEqual(oneLine`${output}`);
});
});
});
4 changes: 3 additions & 1 deletion packages/angular_devkit/build_optimizer/src/transforms/scrub-file.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,9 @@ function isCtorParamsAssignmentExpression(exprStmt: ts.ExpressionStatement): boo
if (expr.operatorToken.kind !== ts.SyntaxKind.FirstAssignment) {
return false;
}
if (expr.right.kind !== ts.SyntaxKind.FunctionExpression) {
if (expr.right.kind !== ts.SyntaxKind.FunctionExpression
&& expr.right.kind !== ts.SyntaxKind.ArrowFunction
) {
return false;
}

Expand Down
14 changes: 14 additions & 0 deletions packages/angular_devkit/build_optimizer/src/transforms/scrub-file_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,20 @@ describe('scrub-file', () => {
expect(oneLine`${transform(input)}`).toEqual(oneLine`${output}`);
});


fit('removes top-level Angular constructor parameters in es2015', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

fit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in #170

const output = stripIndent`
class Clazz extends BaseClazz { constructor(e) { super(e); } }
`;
const input = stripIndent`
${output}
Clazz.ctorParameters = () => [ { type: Injectable } ];
`;

expect(testScrubFile(input)).toBeTruthy();
expect(oneLine`${transform(input)}`).toEqual(oneLine`${output}`);
});

it('removes nested constructor parameters', () => {
const output = stripIndent`
import { Injector } from '@angular/core';
Expand Down
1 change: 1 addition & 0 deletions scripts/validate-commits.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ export default function (_: {}, logger: Logger) {

// Types that MUST NOT contain a scope.
case 'build':
case 'revert':
case 'ci':
if (scope) {
_invalid(sha, message, 'should not have a scope');
Expand Down