Skip to content

Commit

Permalink
feat(@schematics/angular): enable stricter type checking and optimiza…
Browse files Browse the repository at this point in the history
…tion effective coding rules

With this change we enable stricter type checking and optimization effective coding rules when using the `--strict` option.

Changes in schematics
- `ng-new`: A prompt for the `--strict` option was added. This option is a proxy and will be passed to the application and workspace schematics.
- `application`: A `package.json` was added in the `app` folder, to tell the bundlers whether the application is free from side-effect code. When `strict` is `true`. the `sideEffects` will be set `false`.
- `workspace` When `strict` is true, we add stricter TypeScript and Angular type-checking options.

Note: AIO is already using these strict TypeScript compiler settings. PR to enable `strictTemplates`  angular/angular#36391

Reference: TOOL-1366
  • Loading branch information
alan-agius4 authored and dgp1130 committed May 5, 2020
1 parent 2d48ab3 commit cbf0feb
Show file tree
Hide file tree
Showing 22 changed files with 192 additions and 27 deletions.
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
4 changes: 3 additions & 1 deletion tests/legacy-cli/e2e/tests/packages/webpack/test-app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@ export default async function (skipCleaning: () => void) {
await createProjectFromAsset('webpack/test-app');
if (isVe) {
await updateJsonFile('tsconfig.json', config => {
config.angularCompilerOptions.enableIvy = false;
const { angularCompilerOptions = {} } = config;
angularCompilerOptions.enableIvy = false;
config.angularCompilerOptions = angularCompilerOptions;
});
}

Expand Down
3 changes: 3 additions & 0 deletions tests/legacy-cli/e2e/utils/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ export async function createProject(name: string, ...args: string[]) {
// Disable the TS version check to make TS updates easier.
// Only VE does it, but on Ivy the i18n extraction uses VE.
await updateJsonFile('tsconfig.json', config => {
if (!config.angularCompilerOptions) {
config.angularCompilerOptions = {};
}
config.angularCompilerOptions.disableTypeScriptVersionCheck = true;
});
}
Expand Down

0 comments on commit cbf0feb

Please sign in to comment.