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(@ngtools/webpack): support jit mode guarded class metadata removal #19076

Merged
merged 2 commits into from Oct 15, 2020
Merged

fix(@ngtools/webpack): support jit mode guarded class metadata removal #19076

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
300a61c
fix(@ngtools/webpack): support jit mode guarded class metadata removal
clydin Oct 14, 2020
4437968
fix(@angular-devkit/build-optimizer): support jit mode guarded class …
clydin Oct 14, 2020
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
33 changes: 24 additions & 9 deletions packages/angular_devkit/build_optimizer/src/transforms/scrub-file.ts
View file
Open in desktop
Expand Up @@ -49,13 +49,24 @@ function scrubFileTransformer(
}

const exprStmt = node as ts.ExpressionStatement;
const iife = getIifeStatement(exprStmt)?.expression;
// Do checks that don't need the typechecker first and bail early.
if (isIvyPrivateCallExpression(exprStmt) || isCtorParamsAssignmentExpression(exprStmt)) {
if (isCtorParamsAssignmentExpression(exprStmt)) {
nodes.push(node);
} else if (iife && isIvyPrivateCallExpression(iife, 'ɵsetClassMetadata')) {
nodes.push(node);
} else if (
iife &&
ts.isBinaryExpression(iife) &&
isIvyPrivateCallExpression(iife.right, 'ɵsetClassMetadata')
) {
nodes.push(node);
} else if (isDecoratorAssignmentExpression(exprStmt)) {
nodes.push(...pickDecorationNodesToRemove(exprStmt, ngMetadata, checker));
} else if (isDecorateAssignmentExpression(exprStmt, tslibImports, checker)
|| isAngularDecoratorExpression(exprStmt, ngMetadata, tslibImports, checker)) {
} else if (
isDecorateAssignmentExpression(exprStmt, tslibImports, checker) ||
isAngularDecoratorExpression(exprStmt, ngMetadata, tslibImports, checker)
) {
nodes.push(...pickDecorateNodesToRemove(exprStmt, tslibImports, ngMetadata, checker));
} else if (isPropDecoratorAssignmentExpression(exprStmt)) {
nodes.push(...pickPropDecorationNodesToRemove(exprStmt, ngMetadata, checker));
Expand Down Expand Up @@ -299,8 +310,8 @@ function isAssignmentExpressionTo(exprStmt: ts.ExpressionStatement, name: string
return true;
}

function isIvyPrivateCallExpression(exprStmt: ts.ExpressionStatement) {
// Each Ivy private call expression is inside an IIFE as single statements, so we must go down it.
// Each Ivy private call expression is inside an IIFE
function getIifeStatement(exprStmt: ts.ExpressionStatement): null | ts.ExpressionStatement {
const expression = exprStmt.expression;
if (!expression || !ts.isCallExpression(expression) || expression.arguments.length !== 0) {
return null;
Expand All @@ -326,18 +337,22 @@ function isIvyPrivateCallExpression(exprStmt: ts.ExpressionStatement) {
return null;
}

return innerExprStmt;
}

function isIvyPrivateCallExpression(expression: ts.Expression, name: string) {
// Now we're in the IIFE and have the inner expression statement. We can check if it matches
// a private Ivy call.
const callExpr = innerExprStmt.expression;
if (!ts.isCallExpression(callExpr)) {
if (!ts.isCallExpression(expression)) {
return false;
}
const propAccExpr = callExpr.expression;

const propAccExpr = expression.expression;
if (!ts.isPropertyAccessExpression(propAccExpr)) {
return false;
}

if (propAccExpr.name.text !== 'ɵsetClassMetadata') {
if (propAccExpr.name.text != name) {
return false;
}

Expand Down
23 changes: 22 additions & 1 deletion packages/angular_devkit/build_optimizer/src/transforms/scrub-file_spec.ts
View file
Open in desktop
Expand Up @@ -855,7 +855,7 @@ describe('scrub-file', () => {
});

describe('Ivy', () => {
it('removes ɵsetClassMetadata call', () => {
it('removes ɵsetClassMetadata call with pure annotation', () => {
const output = tags.stripIndent`
import { Component } from '@angular/core';
${clazz}
Expand All @@ -875,5 +875,26 @@ describe('scrub-file', () => {
expect(testScrubFile(input)).toBeTruthy();
expect(tags.oneLine`${transform(input)}`).toEqual(tags.oneLine`${output}`);
});

it('removes ɵsetClassMetadata call', () => {
const output = tags.stripIndent`
import { Component } from '@angular/core';
${clazz}
`;
const input = tags.stripIndent`
${output}
(function () { (typeof ngJitMode === "undefined" || ngJitMode) && i0.ɵsetClassMetadata(Clazz, [{
type: Component,
args: [{
selector: 'app-lazy',
template: 'very lazy',
styles: []
}]
}], null, null); })();
`;

expect(testScrubFile(input)).toBeTruthy();
expect(tags.oneLine`${transform(input)}`).toEqual(tags.oneLine`${output}`);
});
});
});
26 changes: 21 additions & 5 deletions packages/ngtools/webpack/src/transformers/remove-ivy-jit-support-calls.ts
View file
Open in desktop
Expand Up @@ -25,14 +25,30 @@ export function removeIvyJitSupportCalls(
return false;
}

let shouldRemove = false;

if (ngModuleScope && ts.isBinaryExpression(innerStatement.expression)) {
return isIvyPrivateCallExpression(innerStatement.expression.right, 'ɵɵsetNgModuleScope');
shouldRemove = isIvyPrivateCallExpression(
innerStatement.expression.right,
'ɵɵsetNgModuleScope',
);
}

if (classMetadata && !shouldRemove) {
if (ts.isBinaryExpression(innerStatement.expression)) {
shouldRemove = isIvyPrivateCallExpression(
innerStatement.expression.right,
'ɵsetClassMetadata',
);
} else {
shouldRemove = isIvyPrivateCallExpression(
innerStatement.expression,
'ɵsetClassMetadata',
);
}
}

return (
classMetadata &&
isIvyPrivateCallExpression(innerStatement.expression, 'ɵsetClassMetadata')
);
return shouldRemove;
})
.forEach(statement => ops.push(new RemoveNodeOperation(sourceFile, statement)));

Expand Down
150 changes: 145 additions & 5 deletions packages/ngtools/webpack/src/transformers/remove-ivy-jit-support-calls_spec.ts
View file
Open in desktop
Expand Up @@ -51,9 +51,39 @@ const input = tags.stripIndent`
}], null, null); })();
`;

const inputNoPure = tags.stripIndent`
export class AppModule {
}
AppModule.ɵmod = i0.ɵɵdefineNgModule({ type: AppModule, bootstrap: [AppComponent] });
AppModule.ɵinj = i0.ɵɵdefineInjector({ factory: function AppModule_Factory(t) { return new (t || AppModule)(); }, providers: [], imports: [[
BrowserModule,
AppRoutingModule
]] });
(function () { (typeof ngJitMode === "undefined" || ngJitMode) && i0.ɵɵsetNgModuleScope(AppModule, { declarations: [AppComponent,
ExampleComponent], imports: [BrowserModule,
AppRoutingModule] }); })();
(function () { (typeof ngJitMode === "undefined" || ngJitMode) && i0.ɵsetClassMetadata(AppModule, [{
type: NgModule,
args: [{
declarations: [
AppComponent,
ExampleComponent
],
imports: [
BrowserModule,
AppRoutingModule
],
providers: [],
bootstrap: [AppComponent]
}]
}], null, null); })();
`;

// tslint:disable-next-line: no-big-function
describe('@ngtools/webpack transformers', () => {
// tslint:disable-next-line: no-big-function
describe('remove-ivy-dev-calls', () => {
it('should allow removing only set class metadata', () => {
it('should allow removing only set class metadata with pure annotation', () => {
const output = tags.stripIndent`
export class AppModule {
}
Expand All @@ -74,7 +104,28 @@ describe('@ngtools/webpack transformers', () => {
expect(tags.oneLine`${result}`).toEqual(tags.oneLine`${output}`);
});

it('should allow removing only ng module scope', () => {
it('should allow removing only set class metadata', () => {
const output = tags.stripIndent`
export class AppModule {
}
AppModule.ɵmod = i0.ɵɵdefineNgModule({ type: AppModule, bootstrap: [AppComponent] });
AppModule.ɵinj = i0.ɵɵdefineInjector({ factory: function AppModule_Factory(t) { return new (t || AppModule)(); }, providers: [], imports: [[
BrowserModule,
AppRoutingModule
]] });
(function () { (typeof ngJitMode === "undefined" || ngJitMode) && i0.ɵɵsetNgModuleScope(AppModule, { declarations: [AppComponent,
ExampleComponent], imports: [BrowserModule,
AppRoutingModule] }); })();
`;

const result = transform(inputNoPure, getTypeChecker =>
removeIvyJitSupportCalls(true, false, getTypeChecker),
);

expect(tags.oneLine`${result}`).toEqual(tags.oneLine`${output}`);
});

it('should allow removing only ng module scope with pure annotation', () => {
const output = tags.stripIndent`
export class AppModule {
}
Expand Down Expand Up @@ -107,7 +158,40 @@ describe('@ngtools/webpack transformers', () => {
expect(tags.oneLine`${result}`).toEqual(tags.oneLine`${output}`);
});

it('should allow removing both set class metadata and ng module scope', () => {
it('should allow removing only ng module scope', () => {
const output = tags.stripIndent`
export class AppModule {
}
AppModule.ɵmod = i0.ɵɵdefineNgModule({ type: AppModule, bootstrap: [AppComponent] });
AppModule.ɵinj = i0.ɵɵdefineInjector({ factory: function AppModule_Factory(t) { return new (t || AppModule)(); }, providers: [], imports: [[
BrowserModule,
AppRoutingModule
]] });
(function () { (typeof ngJitMode === "undefined" || ngJitMode) && i0.ɵsetClassMetadata(AppModule, [{
type: NgModule,
args: [{
declarations: [
AppComponent,
ExampleComponent
],
imports: [
BrowserModule,
AppRoutingModule
],
providers: [],
bootstrap: [AppComponent]
}]
}], null, null); })();
`;

const result = transform(inputNoPure, getTypeChecker =>
removeIvyJitSupportCalls(false, true, getTypeChecker),
);

expect(tags.oneLine`${result}`).toEqual(tags.oneLine`${output}`);
});

it('should allow removing both set class metadata and ng module scope with pure annotation', () => {
const output = tags.stripIndent`
export class AppModule {
}
Expand All @@ -125,15 +209,41 @@ describe('@ngtools/webpack transformers', () => {
expect(tags.oneLine`${result}`).toEqual(tags.oneLine`${output}`);
});

it('should allow removing neither set class metadata nor ng module scope', () => {
it('should allow removing both set class metadata and ng module scope', () => {
const output = tags.stripIndent`
export class AppModule {
}
AppModule.ɵmod = i0.ɵɵdefineNgModule({ type: AppModule, bootstrap: [AppComponent] });
AppModule.ɵinj = i0.ɵɵdefineInjector({ factory: function AppModule_Factory(t) { return new (t || AppModule)(); }, providers: [], imports: [[
BrowserModule,
AppRoutingModule
]] });
`;

const result = transform(inputNoPure, getTypeChecker =>
removeIvyJitSupportCalls(true, true, getTypeChecker),
);

expect(tags.oneLine`${result}`).toEqual(tags.oneLine`${output}`);
});

it('should allow removing neither set class metadata nor ng module scope with pure annotation', () => {
const result = transform(input, getTypeChecker =>
removeIvyJitSupportCalls(false, false, getTypeChecker),
);

expect(tags.oneLine`${result}`).toEqual(tags.oneLine`${input}`);
});

it('should strip unused imports when removing set class metadata and ng module scope', () => {
it('should allow removing neither set class metadata nor ng module scope', () => {
const result = transform(inputNoPure, getTypeChecker =>
removeIvyJitSupportCalls(false, false, getTypeChecker),
);

expect(tags.oneLine`${result}`).toEqual(tags.oneLine`${inputNoPure}`);
});

it('should strip unused imports when removing set class metadata and ng module scope with pure annotation', () => {
const imports = tags.stripIndent`
import { BrowserModule } from '@angular/platform-browser';
import { NgModule } from '@angular/core';
Expand Down Expand Up @@ -164,5 +274,35 @@ describe('@ngtools/webpack transformers', () => {
expect(tags.oneLine`${result}`).toEqual(tags.oneLine`${output}`);
});

it('should strip unused imports when removing set class metadata and ng module scope', () => {
const imports = tags.stripIndent`
import { BrowserModule } from '@angular/platform-browser';
import { NgModule } from '@angular/core';
import { AppRoutingModule } from './app-routing.module';
import { AppComponent } from './app.component';
import { ExampleComponent } from './example/example.component';
import * as i0 from "@angular/core";
`;

const output = tags.stripIndent`
import { BrowserModule } from '@angular/platform-browser';
import { AppRoutingModule } from './app-routing.module';
import { AppComponent } from './app.component';
import * as i0 from "@angular/core";
export class AppModule {
}
AppModule.ɵmod = i0.ɵɵdefineNgModule({ type: AppModule, bootstrap: [AppComponent] });
AppModule.ɵinj = i0.ɵɵdefineInjector({ factory: function AppModule_Factory(t) { return new (t || AppModule)(); }, providers: [], imports: [[
BrowserModule,
AppRoutingModule
]] });
`;

const result = transform(imports + inputNoPure, getTypeChecker =>
removeIvyJitSupportCalls(true, true, getTypeChecker),
);

expect(tags.oneLine`${result}`).toEqual(tags.oneLine`${output}`);
});
});
});