Skip to content

Commit

Permalink
fix: disable injectable-pipe migration (#30180)
Browse files Browse the repository at this point in the history
Disables the injectable pipe migration until we can decide whether this is the right solution for Ivy. Rolling it out properly will involve a more detailed plan and more changes like updating the styleguide, scaffolding schematics etc.

Context for the new `test-migrations.json`: since we use the `migrations.json` both for the real migrations and for tests, it doesn't allow us to disable a schematic, but continue running its tests. This change adds the test-specific file so that we can continue running the `injectable-pipe` tests, even though the schematic itself is disabled.

PR Close #30180
  • Loading branch information
crisbeto authored and AndrewKushnir committed Apr 29, 2019
1 parent b4d291a commit 4b2fcfd
Show file tree
Hide file tree
Showing 9 changed files with 234 additions and 220 deletions.
1 change: 1 addition & 0 deletions packages/core/schematics/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ load("//tools:defaults.bzl", "npm_package")
exports_files([
"tsconfig.json",
"migrations.json",
"test-migrations.json",
])

npm_package(
Expand Down
5 changes: 0 additions & 5 deletions packages/core/schematics/migrations.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,6 @@
"version": "8-beta",
"description": "Warns developers if values are assigned to template variables",
"factory": "./migrations/template-var-assignment/index"
},
"migration-v8-injectable-pipe": {
"version": "8-beta",
"description": "Migrates all Pipe classes so that they have an Injectable annotation",
"factory": "./migrations/injectable-pipe/index"
}
}
}
20 changes: 20 additions & 0 deletions packages/core/schematics/test-migrations.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
{
"schematics": {
"migration-move-document": {
"description": "Migrates DOCUMENT Injection token from platform-browser imports to common import",
"factory": "./migrations/move-document/index"
},
"migration-static-queries": {
"description": "Migrates ViewChild and ContentChild to explicit query timing",
"factory": "./migrations/static-queries/index"
},
"migration-template-local-variables": {
"description": "Warns developers if values are assigned to template variables",
"factory": "./migrations/template-var-assignment/index"
},
"migration-injectable-pipe": {
"description": "Migrates all Pipe classes so that they have an Injectable annotation",
"factory": "./migrations/injectable-pipe/index"
}
}
}
2 changes: 1 addition & 1 deletion packages/core/schematics/test/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ ts_library(
testonly = True,
srcs = glob(["**/*.ts"]),
data = [
"//packages/core/schematics:migrations.json",
"//packages/core/schematics:test-migrations.json",
],
deps = [
"//packages/core/schematics/migrations/injectable-pipe",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ describe('injectable pipe migration', () => {
let previousWorkingDir: string;

beforeEach(() => {
runner = new SchematicTestRunner('test', require.resolve('../migrations.json'));
runner = new SchematicTestRunner('test', require.resolve('../test-migrations.json'));
host = new TempScopedNodeJsSyncHost();
tree = new UnitTestTree(new HostTree(host));

Expand Down Expand Up @@ -123,5 +123,5 @@ describe('injectable pipe migration', () => {
host.sync.write(normalize(filePath), virtualFs.stringToFileBuffer(contents));
}

function runMigration() { runner.runSchematic('migration-v8-injectable-pipe', {}, tree); }
function runMigration() { runner.runSchematic('migration-injectable-pipe', {}, tree); }
});
4 changes: 2 additions & 2 deletions packages/core/schematics/test/move_document_migration_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ describe('move-document migration', () => {
let previousWorkingDir: string;

beforeEach(() => {
runner = new SchematicTestRunner('test', require.resolve('../migrations.json'));
runner = new SchematicTestRunner('test', require.resolve('../test-migrations.json'));
host = new TempScopedNodeJsSyncHost();
tree = new UnitTestTree(new HostTree(host));

Expand Down Expand Up @@ -151,5 +151,5 @@ describe('move-document migration', () => {
host.sync.write(normalize(filePath), virtualFs.stringToFileBuffer(contents));
}

function runMigration() { runner.runSchematic('migration-v8-move-document', {}, tree); }
function runMigration() { runner.runSchematic('migration-move-document', {}, tree); }
});
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ describe('static-queries migration with template strategy', () => {
let warnOutput: string[];

beforeEach(() => {
runner = new SchematicTestRunner('test', require.resolve('../migrations.json'));
runner = new SchematicTestRunner('test', require.resolve('../test-migrations.json'));
host = new TempScopedNodeJsSyncHost();
tree = new UnitTestTree(new HostTree(host));

Expand Down Expand Up @@ -97,15 +97,15 @@ describe('static-queries migration with template strategy', () => {
}

async function runMigration() {
await runner.runSchematicAsync('migration-v8-static-queries', {}, tree).toPromise();
await runner.runSchematicAsync('migration-static-queries', {}, tree).toPromise();
}

describe('ViewChild', () => {

it('should detect queries selecting elements through template reference', async() => {
writeFile('/index.ts', `
import {Component, NgModule, ViewChild} from '@angular/core';
@Component({template: \`
<ng-template>
<button #myButton>My Button</button>
Expand All @@ -118,7 +118,7 @@ describe('static-queries migration with template strategy', () => {
private @ViewChild('myButton') query: any;
private @ViewChild('myStaticButton') query2: any;
}
@NgModule({declarations: [MyComp]})
export class MyModule {}
`);
Expand All @@ -134,7 +134,7 @@ describe('static-queries migration with template strategy', () => {
it('should detect queries selecting ng-template as static', async() => {
writeFile('/index.ts', `
import {Component, NgModule, ViewChild} from '@angular/core';
@Component({template: \`
<ng-template #myTmpl>
My template
Expand All @@ -143,7 +143,7 @@ describe('static-queries migration with template strategy', () => {
export class MyComp {
private @ViewChild('myTmpl') query: any;
}
@NgModule({declarations: [MyComp]})
export class MyModule {}
`);
Expand All @@ -157,29 +157,29 @@ describe('static-queries migration with template strategy', () => {
it('should detect queries selecting component view providers through string token', async() => {
writeFile('/index.ts', `
import {Component, Directive, NgModule, ViewChild} from '@angular/core';
@Directive({
selector: '[myDirective]',
providers: [
{provide: 'my-token', useValue: 'test'}
]
})
export class MyDirective {}
@Directive({
selector: '[myDirective2]',
providers: [
{provide: 'my-token-2', useValue: 'test'}
]
})
export class MyDirective2 {}
@Component({templateUrl: './my-tmpl.html'})
export class MyComp {
private @ViewChild('my-token') query: any;
private @ViewChild('my-token-2') query2: any;
}
@NgModule({declarations: [MyComp, MyDirective, MyDirective2]})
export class MyModule {}
`);
Expand All @@ -202,28 +202,28 @@ describe('static-queries migration with template strategy', () => {
it('should detect queries selecting component view providers using class token', async() => {
writeFile('/index.ts', `
import {Component, Directive, NgModule, ViewChild} from '@angular/core';
export class MyService {}
export class MyService2 {}
@Directive({
selector: '[myDirective]',
providers: [MyService]
})
export class MyDirective {}
@Directive({
selector: '[myDirective2]',
providers: [MyService2]
})
export class MyDirective2 {}
@Component({templateUrl: './my-tmpl.html'})
export class MyComp {
private @ViewChild(MyService) query: any;
private @ViewChild(MyService2) query2: any;
}
@NgModule({declarations: [MyComp, MyDirective, MyDirective2]})
export class MyModule {}
`);
Expand All @@ -247,7 +247,7 @@ describe('static-queries migration with template strategy', () => {
writeFile('/index.ts', `
import {Component, NgModule, ViewChild} from '@angular/core';
import {HomeComponent, HomeComponent2} from './home-comp';
@Component({
template: \`
<home-comp></home-comp>
Expand All @@ -260,20 +260,20 @@ describe('static-queries migration with template strategy', () => {
private @ViewChild(HomeComponent) query: any;
private @ViewChild(HomeComponent2) query2: any;
}
@NgModule({declarations: [MyComp, HomeComponent, HomeComponent2]})
export class MyModule {}
`);

writeFile(`/home-comp.ts`, `
import {Component} from '@angular/core';
@Component({
selector: 'home-comp',
template: '<span>Home</span>'
})
export class HomeComponent {}
@Component({
selector: 'home-comp2',
template: '<span>Home 2</span>'
Expand All @@ -294,12 +294,12 @@ describe('static-queries migration with template strategy', () => {
writeFile('/index.ts', `
import {Component, NgModule, ViewChild} from '@angular/core';
import {MyLibComponent} from 'my-lib';
@Component({templateUrl: './my-tmpl.html'})
export class MyComp {
private @ViewChild(MyLibComponent) query: any;
}
@NgModule({declarations: [MyComp, MyLibComponent]})
export class MyModule {}
`);
Expand All @@ -319,12 +319,12 @@ describe('static-queries migration with template strategy', () => {
writeFile('/index.ts', `
import {Component, NgModule, ViewChild} from '@angular/core';
import {MyLibComponent} from 'my-lib';
@Component({templateUrl: './my-tmpl.html'})
export class MyComp {
private @ViewChild(MyLibComponent) query: any;
}
@NgModule({declarations: [MyComp, MyLibComponent]})
export class MyModule {}
`);
Expand All @@ -345,16 +345,16 @@ describe('static-queries migration with template strategy', () => {
it('should detect queries within structural directive', async() => {
writeFile('/index.ts', `
import {Component, Directive, NgModule, ViewChild} from '@angular/core';
@Directive({selector: '[ngIf]'})
export class FakeNgIf {}
@Component({templateUrl: 'my-tmpl.html'})
export class MyComp {
private @ViewChild('myRef') query: any;
private @ViewChild('myRef2') query2: any;
}
@NgModule({declarations: [MyComp, FakeNgIf]})
export class MyModule {}
`);
Expand All @@ -375,14 +375,14 @@ describe('static-queries migration with template strategy', () => {
it('should detect inherited queries', async() => {
writeFile('/index.ts', `
import {Component, NgModule, ViewChild} from '@angular/core';
export class BaseClass {
@ViewChild('myRef') query: any;
}
@Component({templateUrl: 'my-tmpl.html'})
export class MyComp extends BaseClass {}
@NgModule({declarations: [MyComp]})
export class MyModule {}
`);
Expand All @@ -400,7 +400,7 @@ describe('static-queries migration with template strategy', () => {
it('should add a todo if a query is not declared in any component', async() => {
writeFile('/index.ts', `
import {Component, NgModule, ViewChild, SomeToken} from '@angular/core';
export class NotAComponent {
@ViewChild('myRef', {read: SomeToken}) query: any;
}
Expand All @@ -420,17 +420,17 @@ describe('static-queries migration with template strategy', () => {
it('should add a todo if a query is used multiple times with different timing', async() => {
writeFile('/index.ts', `
import {Component, NgModule, ViewChild} from '@angular/core';
export class BaseClass {
@ViewChild('myRef') query: any;
}
@Component({template: '<ng-template><p #myRef></p></ng-template>'})
export class FirstComp extends BaseClass {}
@Component({template: '<span #myRef></span>'})
export class SecondComp extends BaseClass {}
@NgModule({declarations: [FirstComp, SecondComp]})
export class MyModule {}
`);
Expand All @@ -448,12 +448,12 @@ describe('static-queries migration with template strategy', () => {
it('should gracefully exit migration if queries could not be analyzed', async() => {
writeFile('/index.ts', `
import {Component, ViewChild} from '@angular/core';
@Component({template: '<ng-template><p #myRef></p></ng-template>'})
export class MyComp {
@ViewChild('myRef') query: any;
}
// **NOTE**: Analysis will fail as there is no "NgModule" that declares the component.
`);

Expand Down Expand Up @@ -533,7 +533,7 @@ describe('static-queries migration with template strategy', () => {
writeFile('/src/test.ts', `
import {ViewChild} from '@angular/core';
import {AppComponent} from './app.component';
@Component({template: '<span #test>Test</span>'})
class MyTestComponent {
@ViewChild('test') query: any;
Expand All @@ -542,7 +542,7 @@ describe('static-queries migration with template strategy', () => {

writeFile('/src/app.component.ts', `
import {Component, ViewChild} from '@angular/core';
@Component({template: '<span #test></span>'})
export class AppComponent {
@ViewChild('test') query: any;
Expand All @@ -552,7 +552,7 @@ describe('static-queries migration with template strategy', () => {
writeFile('/src/app.module.ts', `
import {NgModule} from '@angular/core';
import {AppComponent} from './app.component';
@NgModule({declarations: [AppComponent]})
export class MyModule {}
`);
Expand Down
Loading

0 comments on commit 4b2fcfd

Please sign in to comment.