Skip to content

feat(core): initial version of the output migration #57604

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

Closed
pkozlowski-opensource wants to merge 2 commits into from
Closed

feat(core): initial version of the output migration #57604

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
87996da
feat(core): initial version of the output migration
pkozlowski-opensource Aug 29, 2024
5082cb9
fixup! feat(core): initial version of the output migration
pkozlowski-opensource Aug 30, 2024
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
42 changes: 42 additions & 0 deletions packages/core/schematics/migrations/output-migration/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
load("//tools:defaults.bzl", "jasmine_node_test", "ts_library")

ts_library(
name = "migration",
srcs = glob(
["**/*.ts"],
exclude = ["*.spec.ts"],
),
deps = [
"//packages/compiler",
"//packages/compiler-cli",
"//packages/compiler-cli/private",
"//packages/compiler-cli/src/ngtsc/annotations",
"//packages/compiler-cli/src/ngtsc/annotations/directive",
"//packages/compiler-cli/src/ngtsc/imports",
"//packages/compiler-cli/src/ngtsc/metadata",
"//packages/compiler-cli/src/ngtsc/reflection",
"//packages/core/schematics/utils/tsurge",
"@npm//@types/node",
"@npm//typescript",
],
)

ts_library(
name = "test_lib",
testonly = True,
srcs = glob(
["**/*.spec.ts"],
),
deps = [
":migration",
"//packages/compiler-cli",
"//packages/compiler-cli/src/ngtsc/file_system/testing",
"//packages/core/schematics/utils/tsurge",
],
)

jasmine_node_test(
name = "test",
srcs = [":test_lib"],
env = {"FORCE_COLOR": "3"},
)
217 changes: 217 additions & 0 deletions packages/core/schematics/migrations/output-migration/output-migration.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,217 @@
/**
* @license
* Copyright Google LLC 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 {initMockFileSystem} from '../../../../compiler-cli/src/ngtsc/file_system/testing';
import {runTsurgeMigration} from '../../utils/tsurge/testing';
import {absoluteFrom} from '@angular/compiler-cli';
import {OutputMigration} from './output-migration';

describe('outputs', () => {
beforeEach(() => {
initMockFileSystem('Native');
});

describe('outputs migration', () => {
describe('EventEmitter declarations without problematic access patterns', () => {
it('should migrate declaration with a primitive type hint', () => {
verifyDeclaration({
before: '@Output() readonly someChange = new EventEmitter<string>();',
after: 'readonly someChange = output<string>();',
});
});

it('should migrate declaration with complex type hint', () => {
verifyDeclaration({
before: '@Output() readonly someChange = new EventEmitter<string | number>();',
after: 'readonly someChange = output<string | number>();',
});
});

it('should migrate declaration without type hint', () => {
verifyDeclaration({
before: '@Output() readonly someChange = new EventEmitter();',
after: 'readonly someChange = output();',
});
});

it('should take alias into account', () => {
verifyDeclaration({
before: `@Output({alias: 'otherChange'}) readonly someChange = new EventEmitter();`,
after: `readonly someChange = output({ alias: 'otherChange' });`,
});
});

it('should support alias as statically analyzable reference', () => {
verify({
before: `
import {Directive, Output, EventEmitter} from '@angular/core';

const aliasParam = { alias: 'otherChange' } as const;

@Directive()
export class TestDir {
@Output(aliasParam) someChange = new EventEmitter();
}
`,
after: `
import { Directive, output } from '@angular/core';

const aliasParam = { alias: 'otherChange' } as const;

@Directive()
export class TestDir {
readonly someChange = output(aliasParam);
}
`,
});
});

it('should add readonly modifier', () => {
verifyDeclaration({
before: '@Output() someChange = new EventEmitter();',
after: 'readonly someChange = output();',
});
});

it('should respect visibility modifiers', () => {
verifyDeclaration({
before: `@Output() protected someChange = new EventEmitter();`,
after: `protected readonly someChange = output();`,
});
});

it('should migrate multiple outputs', () => {
// TODO: whitespace are messing up test verification
verifyDeclaration({
before: `@Output() someChange1 = new EventEmitter();
@Output() someChange2 = new EventEmitter();`,
after: `readonly someChange1 = output();
readonly someChange2 = output();`,
});
});

it('should migrate only EventEmitter outputs when multiple outputs exist', () => {
// TODO: whitespace are messing up test verification
verifyDeclaration({
before: `@Output() someChange1 = new EventEmitter();
@Output() someChange2 = new Subject();`,
after: `readonly someChange1 = output();
@Output() someChange2 = new Subject();`,
});
});
});

describe('declarations _with_ problematic access patterns', () => {
it('should _not_ migrate outputs that are used with .pipe', () => {
verifyNoChange(`
import {Directive, Output, EventEmitter} from '@angular/core';

@Directive()
export class TestDir {
@Output() someChange = new EventEmitter();

someMethod() {
this.someChange.pipe();
}
}
`);
});

it('should _not_ migrate outputs that are used with .next', () => {
verifyNoChange(`
import {Directive, Output, EventEmitter} from '@angular/core';

@Directive()
export class TestDir {
@Output() someChange = new EventEmitter<string>();

someMethod() {
this.someChange.next('payload');
}
}
`);
});

it('should _not_ migrate outputs that are used with .complete', () => {
verifyNoChange(`
import {Directive, Output, EventEmitter, OnDestroy} from '@angular/core';

@Directive()
export class TestDir implements OnDestroy {
@Output() someChange = new EventEmitter<string>();

ngOnDestroy() {
this.someChange.complete();
}
}
`);
});
});
});

describe('declarations other than EventEmitter', () => {
it('should _not_ migrate outputs that are initialized with sth else than EventEmitter', () => {
verify({
before: populateDeclarationTestCase('@Output() protected someChange = new Subject();'),
after: populateDeclarationTestCase('@Output() protected someChange = new Subject();'),
});
});
});
});

async function verifyDeclaration(testCase: {before: string; after: string}) {
verify({
before: populateDeclarationTestCase(testCase.before),
after: populateExpectedResult(testCase.after),
});
}

async function verifyNoChange(beforeAndAfter: string) {
verify({before: beforeAndAfter, after: beforeAndAfter});
}

async function verify(testCase: {before: string; after: string}) {
const fs = await runTsurgeMigration(new OutputMigration(), [
{
name: absoluteFrom('/app.component.ts'),
isProgramRootFile: true,
contents: testCase.before,
},
]);

let actual = fs.readFile(absoluteFrom('/app.component.ts'));

expect(actual).toBe(testCase.after);
}

function populateDeclarationTestCase(declaration: string): string {
return `
import {
Directive,
Output,
EventEmitter,
Subject
} from '@angular/core';

@Directive()
export class TestDir {
${declaration}
}
`;
}

function populateExpectedResult(declaration: string): string {
return `
import { Directive, Subject, output } from '@angular/core';

@Directive()
export class TestDir {
${declaration}
}
`;
}
Loading