Skip to content

feat(@schematics/angular): enable stricter type checking and optimization effective coding rules #17372

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

Merged
merged 2 commits into from
May 5, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 29 additions & 11 deletions packages/schematics/angular/application/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,34 @@ function addAppToWorkspaceFile(options: ApplicationOptions, appDir: string): Rul
}

const sourceRoot = join(normalize(projectRoot), 'src');
let budgets = [];
if (options.strict) {
budgets = [
{
type: 'initial',
maximumWarning: '500kb',
maximumError: '1mb',
},
{
type: 'anyComponentStyle',
maximumWarning: '2kb',
maximumError: '4kb',
},
];
} else {
budgets = [
{
type: 'initial',
maximumWarning: '2mb',
maximumError: '5mb',
},
{
type: 'anyComponentStyle',
maximumWarning: '6kb',
maximumError: '10kb',
},
];
}

const project = {
root: normalize(projectRoot),
Expand Down Expand Up @@ -219,17 +247,7 @@ function addAppToWorkspaceFile(options: ApplicationOptions, appDir: string): Rul
extractLicenses: true,
vendorChunk: false,
buildOptimizer: true,
budgets: [
{
type: 'initial',
maximumWarning: '2mb',
maximumError: '5mb',
},
{
type: 'anyComponentStyle',
maximumWarning: '6kb',
maximumError: '10kb',
}],
budgets,
},
},
},
Expand Down
18 changes: 18 additions & 0 deletions packages/schematics/angular/application/index_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,24 @@ describe('Application Schematic', () => {
});
});

it('sideEffects property should be true when strict mode', async () => {
const options = { ...defaultOptions, projectRoot: '', strict: true };

const tree = await schematicRunner.runSchematicAsync('application', options, workspaceTree)
.toPromise();
const content = JSON.parse(tree.readContent('/src/app/package.json'));
expect(content.sideEffects).toBe(false);
});

it('sideEffects property should be false when not in strict mode', async () => {
const options = { ...defaultOptions, projectRoot: '', strict: false };

const tree = await schematicRunner.runSchematicAsync('application', options, workspaceTree)
.toPromise();
const content = JSON.parse(tree.readContent('/src/app/package.json'));
expect(content.sideEffects).toBe(true);
});

describe('custom projectRoot', () => {
it('should put app files in the right spot', async () => {
const options = { ...defaultOptions, projectRoot: '' };
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "<%= utils.dasherize(name) %>",
"private": true,
"description": "This is a special package.json file that is not used by package managers. It is however used to tell the tools and bundlers whether the code under this directory is free of code with non-local side-effect. Any code that does have non-local side-effects can't be well optimized (tree-shaken) and will result in unnecessary increased payload size. It should be safe to set this option to 'false' for new applications, but existing code bases could be broken when built with the production config if the application code does contain non-local side-effects that the application depends on.",
"sideEffects": <%= !strict %>
}
5 changes: 5 additions & 0 deletions packages/schematics/angular/application/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,11 @@
"description": "When true, applies lint fixes after generating the application.",
"x-user-analytics": 15
},
"strict": {
"description": "Creates an application with stricter build optimization options.",
"type": "boolean",
"default": false
},
"legacyBrowsers": {
"type": "boolean",
"description": "Add support for legacy browsers like Internet Explorer using differential loading.",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,11 @@
"version": "10.0.0-beta.3",
"factory": "./update-10/update-angular-config",
"description": "Remove various deprecated builders options from 'angular.json'."
},
"side-effects-package-json": {
"version": "10.0.0-beta.3",
"factory": "./update-10/side-effects-package-json",
"description": "Create a special 'package.json' file that is used to tell the tools and bundlers whether the code under the app directory is free of code with non-local side-effect."
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/**
* @license
* Copyright Google Inc. All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/

import { join, normalize, strings } from '@angular-devkit/core';
import { Rule } from '@angular-devkit/schematics';
import { getWorkspace } from '../../utility/workspace';
import { ProjectType } from '../../utility/workspace-models';

export default function (): Rule {
return async (host, context) => {
const workspace = await getWorkspace(host);
const logger = context.logger;

for (const [projectName, project] of workspace.projects) {
if (project.extensions.projectType !== ProjectType.Application) {
// Only interested in application projects
continue;
}

const appDir = join(normalize(project.sourceRoot || ''), 'app');
const { subdirs, subfiles } = host.getDir(appDir);
if (!subdirs.length && !subfiles.length) {
logger.error(`Application directory '${appDir}' for project '${projectName}' doesn't exist.`);
continue;
}

const pkgJson = join(appDir, 'package.json');
if (!host.exists(pkgJson)) {
const pkgJsonContent = {
name: strings.dasherize(projectName),
private: true,
description: `This is a special package.json file that is not used by package managers. It is however used to tell the tools and bundlers whether the code under this directory is free of code with non-local side-effect. Any code that does have non-local side-effects can't be well optimized (tree-shaken) and will result in unnecessary increased payload size. It should be safe to set this option to 'false' for new applications, but existing code bases could be broken when built with the production config if the application code does contain non-local side-effects that the application depends on.`,
sideEffects: true,
};

host.create(pkgJson, JSON.stringify(pkgJsonContent, undefined, 2));
}
}
};
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/**
* @license
* Copyright Google Inc. All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
import { EmptyTree } from '@angular-devkit/schematics';
import { SchematicTestRunner, UnitTestTree } from '@angular-devkit/schematics/testing';
import { Builders, ProjectType, WorkspaceSchema } from '../../utility/workspace-models';

function createWorkSpaceConfig(tree: UnitTestTree) {
const angularConfig: WorkspaceSchema = {
version: 1,
projects: {
demo: {
root: '',
sourceRoot: 'src',
projectType: ProjectType.Application,
prefix: 'app',
architect: {
build: {
builder: Builders.Browser,
options: {
tsConfig: '',
main: '',
polyfills: '',
},
},
},
},
},
};

tree.create('/angular.json', JSON.stringify(angularConfig, undefined, 2));
}

describe(`Migration to add sideEffects package.json`, () => {
const schematicName = 'side-effects-package-json';

const schematicRunner = new SchematicTestRunner(
'migrations',
require.resolve('../migration-collection.json'),
);

let tree: UnitTestTree;
beforeEach(() => {
tree = new UnitTestTree(new EmptyTree());
createWorkSpaceConfig(tree);
tree.create('src/app/main.ts', '');
});

it(`should create a package.json with sideEffects true under app folder.`, async () => {
const newTree = await schematicRunner.runSchematicAsync(schematicName, {}, tree).toPromise();
const { name, sideEffects } = JSON.parse(newTree.readContent('src/app/package.json'));
expect(name).toBe('demo');
expect(sideEffects).toBeTrue();
});
});
1 change: 1 addition & 0 deletions packages/schematics/angular/ng-new/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ export default function(options: NgNewOptions): Rule {
skipPackageJson: false,
// always 'skipInstall' here, so that we do it after the move
skipInstall: true,
strict: options.strict,
minimal: options.minimal,
legacyBrowsers: options.legacyBrowsers,
};
Expand Down
9 changes: 5 additions & 4 deletions packages/schematics/angular/ng-new/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -118,20 +118,21 @@
"x-user-analytics": 12
},
"createApplication": {
"description": "When true (the default), creates a new initial app project in the src folder of the new workspace. When false, creates an empty workspace with no initial app. You can then use the generate application command so that all apps are created in the projects folder.",
"description": "When true (the default), creates a new initial application project in the src folder of the new workspace. When false, creates an empty workspace with no initial app. You can then use the generate application command so that all apps are created in the projects folder.",
"type": "boolean",
"default": true
},
"minimal": {
"description": "When true, creates a project without any testing frameworks. (Use for learning purposes only.)",
"description": "When true, creates a workspace without any testing frameworks. (Use for learning purposes only.)",
"type": "boolean",
"default": false,
"x-user-analytics": 14
},
"strict": {
"description": "Creates a workspace with stricter TypeScript compiler options.",
"description": "Creates a workspace with stricter type checking and build optimization options.",
"type": "boolean",
"default": false
"default": false,
"x-prompt": "Create a workspace with stricter type checking and more efficient production optimizations?"
},
"legacyBrowsers": {
"type": "boolean",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,10 @@
"compilerOptions": {
"baseUrl": "./",
"outDir": "./dist/out-tsc",<% if (strict) { %>
"noImplicitAny": true,
"forceConsistentCasingInFileNames": true,
"strict": true,
"noImplicitReturns": true,
"noImplicitThis": true,
"noFallthroughCasesInSwitch": true,
"strictNullChecks": true,<% } %>
"noFallthroughCasesInSwitch": true,<% } %>
"sourceMap": true,
"declaration": false,
"downlevelIteration": true,
Expand All @@ -20,9 +19,9 @@
"es2018",
"dom"
]
},
}<% if (strict) { %>,
"angularCompilerOptions": {
"fullTemplateTypeCheck": true,
"strictInjectionParameters": true
}
"strictInjectionParameters": true,
"strictTemplates": true
}<% } %>
}
10 changes: 6 additions & 4 deletions packages/schematics/angular/workspace/index_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,15 @@ describe('Workspace Schematic', () => {

it('should not add strict compiler options when false', async () => {
const tree = await schematicRunner.runSchematicAsync('workspace', { ...defaultOptions, strict: false }).toPromise();
const { compilerOptions } = JSON.parse(tree.readContent('/tsconfig.json'));
expect(compilerOptions.strictNullChecks).not.toBeDefined();
const { compilerOptions, angularCompilerOptions } = JSON.parse(tree.readContent('/tsconfig.json'));
expect(compilerOptions.strict).toBeUndefined();
expect(angularCompilerOptions).toBeUndefined();
});

it('should not add strict compiler options when true', async () => {
const tree = await schematicRunner.runSchematicAsync('workspace', { ...defaultOptions, strict: true }).toPromise();
const { compilerOptions } = JSON.parse(tree.readContent('/tsconfig.json'));
expect(compilerOptions.strictNullChecks).toBe(true);
const { compilerOptions, angularCompilerOptions } = JSON.parse(tree.readContent('/tsconfig.json'));
expect(compilerOptions.strict).toBe(true);
expect(angularCompilerOptions.strictTemplates).toBe(true);
});
});
2 changes: 1 addition & 1 deletion packages/schematics/angular/workspace/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
"x-user-analytics": 14
},
"strict": {
"description": "Creates a workspace with stricter TypeScript compiler options.",
"description": "Creates a workspace with stricter type checking options.",
"type": "boolean",
"default": false
},
Expand Down
4 changes: 3 additions & 1 deletion tests/legacy-cli/e2e/setup/500-create-project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ export default async function() {

if (argv['ve']) {
await updateJsonFile('tsconfig.json', config => {
config.angularCompilerOptions.enableIvy = false;
const { angularCompilerOptions = {} } = config;
angularCompilerOptions.enableIvy = false;
config.angularCompilerOptions = angularCompilerOptions;
});

// In VE non prod builds are non AOT by default
Expand Down
4 changes: 3 additions & 1 deletion tests/legacy-cli/e2e/tests/basic/ivy-opt-out.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ export default async function() {

// View Engine (NGC) compilation should work after running NGCC from Webpack
await updateJsonFile('tsconfig.json', config => {
config.angularCompilerOptions.enableIvy = false;
const { angularCompilerOptions = {} } = config;
angularCompilerOptions.enableIvy = false;
config.angularCompilerOptions = angularCompilerOptions;
});

// verify that VE compilation works during runtime
Expand Down
7 changes: 7 additions & 0 deletions tests/legacy-cli/e2e/tests/build/strict-mode.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { ng } from '../../utils/process';
import { createProject } from '../../utils/project';

export default async function() {
await createProject('strict-test-project', '--strict');
await ng('e2e', '--prod');
}
7 changes: 0 additions & 7 deletions tests/legacy-cli/e2e/tests/build/strict-workspace.ts

This file was deleted.

3 changes: 3 additions & 0 deletions tests/legacy-cli/e2e/tests/i18n/ivy-localize-dl-xliff2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ export async function executeTest() {

await updateJsonFile('tsconfig.json', config => {
config.compilerOptions.target = 'es2015';
if (!config.angularCompilerOptions) {
config.angularCompilerOptions = {};
}
config.angularCompilerOptions.disableTypeScriptVersionCheck = true;
});

Expand Down
3 changes: 3 additions & 0 deletions tests/legacy-cli/e2e/tests/i18n/ivy-localize-es2015.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ export default async function() {
await writeFile('.browserslistrc', 'Chrome 65');
await updateJsonFile('tsconfig.json', config => {
config.compilerOptions.target = 'es2015';
if (!config.angularCompilerOptions) {
config.angularCompilerOptions = {};
}
config.angularCompilerOptions.disableTypeScriptVersionCheck = true;
});

Expand Down
3 changes: 3 additions & 0 deletions tests/legacy-cli/e2e/tests/i18n/ivy-localize-es5.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ export default async function() {
// Ensure a es5 build is used.
await updateJsonFile('tsconfig.json', config => {
config.compilerOptions.target = 'es5';
if (!config.angularCompilerOptions) {
config.angularCompilerOptions = {};
}
config.angularCompilerOptions.disableTypeScriptVersionCheck = true;
});

Expand Down
3 changes: 3 additions & 0 deletions tests/legacy-cli/e2e/tests/i18n/ivy-localize-serviceworker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ export default async function() {

await updateJsonFile('tsconfig.json', config => {
config.compilerOptions.target = 'es2015';
if (!config.angularCompilerOptions) {
config.angularCompilerOptions = {};
}
config.angularCompilerOptions.disableTypeScriptVersionCheck = true;
});

Expand Down
3 changes: 3 additions & 0 deletions tests/legacy-cli/e2e/tests/i18n/ve-localize-es2015.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ export default async function() {
await writeFile('.browserslistrc', 'Chrome 65');
await updateJsonFile('tsconfig.json', config => {
config.compilerOptions.target = 'es2015';
if (!config.angularCompilerOptions) {
config.angularCompilerOptions = {};
}
config.angularCompilerOptions.disableTypeScriptVersionCheck = true;
});

Expand Down
Loading