fix(core): TypeScript related migrations should cater for BOM (#30719)

fix(@schematics/angular): TypeScript related migrations should cater for BOM

In the CLI `UpdateRecorder` methods such as `insertLeft`, `remove` etc.. accepts positions which are not offset by a BOM. This is because when a file has a BOM a different recorder will be used https://github.com/angular/angular-cli/blob/master/packages/angular_devkit/schematics/src/tree/recorder.ts#L72 which caters for an addition offset/delta.

The main reason for this is that when a developer is writing a schematic they shouldn't need to compute the offset based if a file has a BOM or not and is handled out of the box.

Example
```ts
recorder.insertLeft(5, 'true');
```

However this is unfortunate in the case if a ts SourceFile is used and one uses `getWidth` and `getStart` method they will already be offset by 1, which at the end it results in a double offset and hence the problem.

Fixes #30713

PR Close #30719
This commit is contained in:
Alan Agius 2019-05-29 12:05:49 +02:00 committed by Misko Hevery
parent 99c9bcab03
commit 80394ce08b
8 changed files with 88 additions and 6 deletions

View File

@ -47,7 +47,10 @@ function runInjectablePipeMigration(tree: Tree, tsconfigPath: string, basePath:
// source files, it can end up updating query definitions multiple times. // source files, it can end up updating query definitions multiple times.
host.readFile = fileName => { host.readFile = fileName => {
const buffer = tree.read(relative(basePath, fileName)); const buffer = tree.read(relative(basePath, fileName));
return buffer ? buffer.toString() : undefined; // Strip BOM as otherwise TSC methods (Ex: getWidth) will return an offset which
// which breaks the CLI UpdateRecorder.
// See: https://github.com/angular/angular/pull/30719
return buffer ? buffer.toString().replace(/^\uFEFF/, '') : undefined;
}; };
const program = ts.createProgram(parsed.fileNames, parsed.options, host); const program = ts.createProgram(parsed.fileNames, parsed.options, host);

View File

@ -48,7 +48,10 @@ function runMoveDocumentMigration(tree: Tree, tsconfigPath: string, basePath: st
// source files, it can end up updating query definitions multiple times. // source files, it can end up updating query definitions multiple times.
host.readFile = fileName => { host.readFile = fileName => {
const buffer = tree.read(relative(basePath, fileName)); const buffer = tree.read(relative(basePath, fileName));
return buffer ? buffer.toString() : undefined; // Strip BOM as otherwise TSC methods (Ex: getWidth) will return an offset which
// which breaks the CLI UpdateRecorder.
// See: https://github.com/angular/angular/pull/30719
return buffer ? buffer.toString().replace(/^\uFEFF/, '') : undefined;
}; };
const program = ts.createProgram(parsed.fileNames, parsed.options, host); const program = ts.createProgram(parsed.fileNames, parsed.options, host);

View File

@ -125,7 +125,10 @@ function analyzeProject(
// source files, it can end up updating query definitions multiple times. // source files, it can end up updating query definitions multiple times.
host.readFile = fileName => { host.readFile = fileName => {
const buffer = tree.read(relative(basePath, fileName)); const buffer = tree.read(relative(basePath, fileName));
return buffer ? buffer.toString() : undefined; // Strip BOM as otherwise TSC methods (Ex: getWidth) will return an offset which
// which breaks the CLI UpdateRecorder.
// See: https://github.com/angular/angular/pull/30719
return buffer ? buffer.toString().replace(/^\uFEFF/, '') : undefined;
}; };
const program = ts.createProgram(parsed.fileNames, parsed.options, host); const program = ts.createProgram(parsed.fileNames, parsed.options, host);

View File

@ -53,7 +53,10 @@ function runTemplateVariableAssignmentCheck(
// program to be based on the file contents in the virtual file tree. // program to be based on the file contents in the virtual file tree.
host.readFile = fileName => { host.readFile = fileName => {
const buffer = tree.read(relative(basePath, fileName)); const buffer = tree.read(relative(basePath, fileName));
return buffer ? buffer.toString() : undefined; // Strip BOM as otherwise TSC methods (Ex: getWidth) will return an offset which
// which breaks the CLI UpdateRecorder.
// See: https://github.com/angular/angular/pull/30719
return buffer ? buffer.toString().replace(/^\uFEFF/, '') : undefined;
}; };
const program = ts.createProgram(parsed.fileNames, parsed.options, host); const program = ts.createProgram(parsed.fileNames, parsed.options, host);

View File

@ -60,7 +60,21 @@ describe('injectable pipe migration', () => {
.toMatch(/@Injectable\(\)\s+@Pipe\(\{ name: 'myPipe' \}\)\s+export class MyPipe/); .toMatch(/@Injectable\(\)\s+@Pipe\(\{ name: 'myPipe' \}\)\s+export class MyPipe/);
}); });
it('should add an import for Injectable to the @angular/core import declaration', async() => { it('should add @Injectable to pipes that do not have it (BOM)', () => {
writeFile('/index.ts', `\uFEFF
import { Pipe } from '@angular/core';
@Pipe({ name: 'myPipe' })
export class MyPipe {
}
`);
runMigration();
expect(tree.readContent('/index.ts'))
.toMatch(/@Injectable\(\)\s+@Pipe\(\{ name: 'myPipe' \}\)\s+export class MyPipe/);
});
it('should add an import for Injectable to the @angular/core import declaration', () => {
writeFile('/index.ts', ` writeFile('/index.ts', `
import { Pipe } from '@angular/core'; import { Pipe } from '@angular/core';

View File

@ -60,7 +60,20 @@ describe('move-document migration', () => {
expect(content).not.toContain(`import {DOCUMENT} from '@angular/platform-browser';`); expect(content).not.toContain(`import {DOCUMENT} from '@angular/platform-browser';`);
}); });
it('should properly apply import replacement with existing import', async() => { it('should properly apply import replacement (BOM)', () => {
writeFile('/index.ts', `\uFEFF
import {DOCUMENT} from '@angular/platform-browser';
`);
runMigration();
const content = tree.readContent('/index.ts');
expect(content).toContain(`import { DOCUMENT } from "@angular/common";`);
expect(content).not.toContain(`import {DOCUMENT} from '@angular/platform-browser';`);
});
it('should properly apply import replacement with existing import', () => {
writeFile('/index.ts', ` writeFile('/index.ts', `
import {DOCUMENT} from '@angular/platform-browser'; import {DOCUMENT} from '@angular/platform-browser';
import {someImport} from '@angular/common'; import {someImport} from '@angular/common';

View File

@ -158,6 +158,29 @@ describe('static-queries migration with template strategy', () => {
.toContain(`@ViewChild('myTmpl', { static: true }) query: any;`); .toContain(`@ViewChild('myTmpl', { static: true }) query: any;`);
}); });
it('should detect queries selecting ng-template as static (BOM)', async() => {
writeFile('/index.ts', `\uFEFF
import {Component, NgModule, ViewChild} from '@angular/core';
@Component({template: \`
<ng-template #myTmpl>
My template
</ng-template>
\`})
export class MyComp {
private @ViewChild('myTmpl') query: any;
}
@NgModule({declarations: [MyComp]})
export class MyModule {}
`);
await runMigration();
expect(tree.readContent('/index.ts'))
.toContain(`@ViewChild('myTmpl', { static: true }) query: any;`);
});
it('should detect queries selecting component view providers through string token', async() => { it('should detect queries selecting component view providers through string token', async() => {
writeFile('/index.ts', ` writeFile('/index.ts', `
import {Component, Directive, NgModule, ViewChild} from '@angular/core'; import {Component, Directive, NgModule, ViewChild} from '@angular/core';

View File

@ -127,6 +127,26 @@ describe('static-queries migration with usage strategy', () => {
.toContain(`@ContentChild('test', { static: false }) query: any;`); .toContain(`@ContentChild('test', { static: false }) query: any;`);
}); });
it('should not mark content queries used in "ngAfterContentInit" as static (BOM)', async() => {
writeFile('/index.ts', `\uFEFF
import {Component, ContentChild} from '@angular/core';
@Component({template: '<span #test></span>'})
export class MyComp {
@ContentChild('test') query: any;
ngAfterContentInit() {
this.query.classList.add('test');
}
}
`);
await runMigration();
expect(tree.readContent('/index.ts'))
.toContain(`@ContentChild('test', { static: false }) query: any;`);
});
it('should not mark content queries used in "ngAfterContentChecked" as static', async() => { it('should not mark content queries used in "ngAfterContentChecked" as static', async() => {
writeFile('/index.ts', ` writeFile('/index.ts', `
import {Component, ContentChild} from '@angular/core'; import {Component, ContentChild} from '@angular/core';