Skip to content

Tsconfig migration fixes #16050

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

Merged
merged 2 commits into from
Nov 4, 2019
Merged

Tsconfig migration fixes #16050

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
081f309
fix(@schematics/angular): replace `'**/*.ts` file inclusion with `**/…
alan-agius4 Nov 3, 2019
8c5e71b
fix(@schematics/angular): tsconfig creates invalid file references
alan-agius4 Nov 3, 2019
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 29 additions & 14 deletions packages/schematics/angular/migrations/update-9/update-app-tsconfigs.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/
import { JsonAstObject } from '@angular-devkit/core';
import { Rule, Tree, UpdateRecorder } from '@angular-devkit/schematics';
import { posix } from 'path';
import {
findPropertyInAstObject,
insertPropertyInAstObjectInOrder,
Expand Down Expand Up @@ -78,29 +79,43 @@ function updateTsConfig(tree: Tree, builderConfig: JsonAstObject, builderName: B
// Note: we need to re-read the tsconfig after very commit because
// otherwise the updates will be out of sync since we are ammending the same node.
tsConfigAst = readJsonFileAsAstObject(tree, tsConfigPath);
const files = findPropertyInAstObject(tsConfigAst, 'files');
const include = findPropertyInAstObject(tsConfigAst, 'include');

if (!files && !include) {
const rootInSrc = tsConfigPath.includes('src/');
const rootSrc = rootInSrc ? '' : 'src/';
const files = builderName === Builders.Server
? [`${rootSrc}main.server.ts`]
: [`${rootSrc}main.ts`, `${rootSrc}polyfills.ts`];
if (include && include.kind === 'array') {
const tsInclude = include.elements.find(({ value }) => typeof value === 'string' && value.endsWith('**/*.ts'));
if (tsInclude) {
const { start, end } = tsInclude;
recorder = tree.beginUpdate(tsConfigPath);
recorder.remove(start.offset, end.offset - start.offset);
// Replace ts includes with d.ts
recorder.insertLeft(start.offset, tsInclude.text.replace('.ts', '.d.ts'));
tree.commitUpdate(recorder);
}
}

recorder = tree.beginUpdate(tsConfigPath);
insertPropertyInAstObjectInOrder(recorder, tsConfigAst, 'files', files, 2);
tree.commitUpdate(recorder);
const files = findPropertyInAstObject(tsConfigAst, 'files');
if (!files) {
const newFiles: string[] = [];

if (builderName === Builders.Browser) {
tsConfigAst = readJsonFileAsAstObject(tree, tsConfigPath);
const mainOption = findPropertyInAstObject(option, 'main');
if (mainOption && mainOption.kind === 'string') {
newFiles.push(posix.relative(posix.dirname(tsConfigPath), mainOption.value));
}

const polyfillsOption = findPropertyInAstObject(option, 'polyfills');
if (polyfillsOption && polyfillsOption.kind === 'string') {
newFiles.push(posix.relative(posix.dirname(tsConfigPath), polyfillsOption.value));
}

if (newFiles.length) {
recorder = tree.beginUpdate(tsConfigPath);
insertPropertyInAstObjectInOrder(recorder, tsConfigAst, 'include', [`${rootSrc}**/*.d.ts`], 2);
tsConfigAst = readJsonFileAsAstObject(tree, tsConfigPath);
insertPropertyInAstObjectInOrder(recorder, tsConfigAst, 'files', newFiles, 2);
tree.commitUpdate(recorder);
}

tsConfigAst = readJsonFileAsAstObject(tree, tsConfigPath);
recorder = tree.beginUpdate(tsConfigPath);
tsConfigAst = readJsonFileAsAstObject(tree, tsConfigPath);
removePropertyInAstObject(recorder, tsConfigAst, 'exclude');
tree.commitUpdate(recorder);
}
Expand Down
38 changes: 34 additions & 4 deletions packages/schematics/angular/migrations/update-9/update-app-tsconfigs_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,12 @@ describe('Migration to version 9', () => {
it('should update apps tsConfig with stricter files inclusions', async () => {
overrideJsonFile(tree, 'tsconfig.app.json', defaultTsConfigOptions);
const tree2 = await schematicRunner.runSchematicAsync('migration-09', {}, tree.branch()).toPromise();
const { exclude, files, include } = JSON.parse(tree2.readContent('tsconfig.app.json'));
const { exclude, files } = JSON.parse(tree2.readContent('tsconfig.app.json'));
expect(exclude).toBeUndefined();
expect(files).toEqual(['src/main.ts', 'src/polyfills.ts']);
expect(include).toEqual(['src/**/*.d.ts']);
});

it('should not update apps tsConfig when tsconfig has include', async () => {
it('should update apps tsConfig when tsconfig has include', async () => {
const tsConfigContent = {
...defaultTsConfigOptions,
include: ['foo.ts'],
Expand All @@ -74,10 +73,41 @@ describe('Migration to version 9', () => {

const tree2 = await schematicRunner.runSchematicAsync('migration-09', {}, tree.branch()).toPromise();
const { files, include } = JSON.parse(tree2.readContent('tsconfig.app.json'));
expect(files).toEqual(undefined);
expect(files).toEqual(['src/main.ts', 'src/polyfills.ts']);
expect(include).toEqual(['foo.ts']);
});

it(`should update include '**/*.ts' in apps tsConfig to '**/*.d.ts'`, async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

This migration will cause projects that use the string format of loadChildren with broken compilations, because the lazy routes will no longer be part of the compilation.

That being said, we have already deprecated the string syntax and migrated these as part of the previous version 8 migration so it should be ok. If users have trouble with this we can provide instructions on how to add the lazy routes to the compilation for users that need to still use the string syntax.
const tsConfigContent = {
...defaultTsConfigOptions,
include: ['src/**/*.ts'],
exclude: ['test.ts'],
};

overrideJsonFile(tree, 'tsconfig.app.json', tsConfigContent);

const tree2 = await schematicRunner.runSchematicAsync('migration-09', {}, tree.branch()).toPromise();
const { files, include, exclude } = JSON.parse(tree2.readContent('tsconfig.app.json'));
expect(files).toEqual(['src/main.ts', 'src/polyfills.ts']);
expect(include).toEqual(['src/**/*.d.ts']);
expect(exclude).toBeUndefined();
});

it(`should update include '**/*.ts' in apps tsConfig to '**/*.d.ts' when includes contains multiple elements`, async () => {
const tsConfigContent = {
...defaultTsConfigOptions,
include: ['foo.ts', 'src/**/*.ts'],
};

overrideJsonFile(tree, 'tsconfig.app.json', tsConfigContent);

const tree2 = await schematicRunner.runSchematicAsync('migration-09', {}, tree.branch()).toPromise();
const { files, include, exclude } = JSON.parse(tree2.readContent('tsconfig.app.json'));
expect(files).toEqual(['src/main.ts', 'src/polyfills.ts']);
expect(include).toEqual(['foo.ts', 'src/**/*.d.ts']);
expect(exclude).toBeUndefined();
});

it(`should remove angularCompilerOptions when enableIvy is true and it's the only option`, async () => {
overrideJsonFile(tree, 'tsconfig.app.json', defaultTsConfigOptions);
const tree2 = await schematicRunner.runSchematicAsync('migration-09', {}, tree.branch()).toPromise();
Expand Down