Skip to content

Commit 6ce4b6b

Browse files
JeanMecheChellappanRajan
JeanMeche
authored and
ChellappanRajan
committed
refactor(core): handle angular#24571 todos. (angular#49221)
This commit removes the remaining TODO(issue/24571) in core code base. PR Close angular#49221
1 parent 292d638 commit 6ce4b6b

20 files changed

+127
-269
lines changed

‎packages/core/src/change_detection/differs/default_iterable_differ.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ const trackByIdentity = (index: number, item: any) => item;
3232
*/
3333
export class DefaultIterableDiffer<V> implements IterableDiffer<V>, IterableChanges<V> {
3434
public readonly length: number = 0;
35-
// TODO(issue/24571): remove '!'.
35+
// TODO: confirm the usage of `collection` as it's unused, readonly and on a non public API.
3636
public readonly collection!: V[]|Iterable<V>|null;
3737
// Keeps track of the used records at any point in time (during & across `_check()` calls)
3838
private _linkedRecords: _DuplicateMap<V>|null = null;
@@ -217,7 +217,8 @@ export class DefaultIterableDiffer<V> implements IterableDiffer<V>, IterableChan
217217
}
218218

219219
this._truncate(record);
220-
(this as {collection: V[] | Iterable<V>}).collection = collection;
220+
// @ts-expect-error overwriting a readonly member
221+
this.collection = collection;
221222
return this.isDirty;
222223
}
223224

‎packages/core/src/render/api.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,7 @@ export abstract class Renderer2 {
9494
* If null or undefined, the view engine won't call it.
9595
* This is used as a performance optimization for production mode.
9696
*/
97-
// TODO(issue/24571): remove '!'.
98-
destroyNode!: ((node: any) => void)|null;
97+
destroyNode: ((node: any) => void)|null = null;
9998
/**
10099
* Appends a child to a given parent node in the host element DOM.
101100
* @param parent The parent node.

‎packages/core/test/acceptance/i18n_spec.ts

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2441,20 +2441,16 @@ describe('runtime i18n', () => {
24412441
it('detached nodes should still be part of query', () => {
24422442
@Directive({selector: '[text]', inputs: ['text'], exportAs: 'textDir'})
24432443
class TextDirective {
2444-
// TODO(issue/24571): remove '!'.
2445-
text!: string;
2444+
text: string|undefined;
24462445
constructor() {}
24472446
}
24482447

24492448
@Component({selector: 'div-query', template: '<ng-container #vc></ng-container>'})
24502449
class DivQuery {
2451-
// TODO(issue/24571): remove '!'.
24522450
@ContentChild(TemplateRef, {static: true}) template!: TemplateRef<any>;
24532451

2454-
// TODO(issue/24571): remove '!'.
24552452
@ViewChild('vc', {read: ViewContainerRef, static: true}) vc!: ViewContainerRef;
24562453

2457-
// TODO(issue/24571): remove '!'.
24582454
@ContentChildren(TextDirective, {descendants: true}) query!: QueryList<TextDirective>;
24592455

24602456
create() {
@@ -2836,13 +2832,13 @@ describe('runtime i18n', () => {
28362832
child = this;
28372833
}
28382834
}
2839-
let child!: Child;
2835+
let child: Child|undefined;
28402836

28412837

28422838
TestBed.configureTestingModule({declarations: [MyApp, Parent, Middle, Child]});
28432839
const fixture = TestBed.createComponent(MyApp);
28442840
fixture.detectChanges();
2845-
expect(child.middle).toBeInstanceOf(Middle);
2841+
expect(child?.middle).toBeInstanceOf(Middle);
28462842
});
28472843

28482844
it('should allow container in gotClosestRElement', () => {

‎packages/core/test/animation/animation_integration_spec.ts

Lines changed: 41 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -1850,8 +1850,7 @@ describe('animation tests', function() {
18501850
]
18511851
})
18521852
class Cmp {
1853-
// TODO(issue/24571): remove '!'.
1854-
public exp!: string|null;
1853+
public exp: string|null|undefined;
18551854
}
18561855

18571856
TestBed.configureTestingModule({declarations: [Cmp]});
@@ -2236,8 +2235,7 @@ describe('animation tests', function() {
22362235
})
22372236
class Cmp {
22382237
public exp: any;
2239-
// TODO(issue/24571): remove '!'.
2240-
public color!: string|null;
2238+
public color: string|null|undefined;
22412239
}
22422240

22432241
TestBed.configureTestingModule({declarations: [Cmp]});
@@ -2737,8 +2735,7 @@ describe('animation tests', function() {
27372735
})
27382736
class Cmp {
27392737
exp: any = false;
2740-
// TODO(issue/24571): remove '!'.
2741-
event!: AnimationEvent;
2738+
event: AnimationEvent|undefined;
27422739

27432740
callback = (event: any) => {
27442741
this.event = event;
@@ -2754,11 +2751,11 @@ describe('animation tests', function() {
27542751
fixture.detectChanges();
27552752
flushMicrotasks();
27562753

2757-
expect(cmp.event.triggerName).toEqual('myAnimation');
2758-
expect(cmp.event.phaseName).toEqual('start');
2759-
expect(cmp.event.totalTime).toEqual(500);
2760-
expect(cmp.event.fromState).toEqual('void');
2761-
expect(cmp.event.toState).toEqual('true');
2754+
expect(cmp.event?.triggerName).toEqual('myAnimation');
2755+
expect(cmp.event?.phaseName).toEqual('start');
2756+
expect(cmp.event?.totalTime).toEqual(500);
2757+
expect(cmp.event?.fromState).toEqual('void');
2758+
expect(cmp.event?.toState).toEqual('true');
27622759
}));
27632760

27642761
it('should trigger a `done` state change listener for when the animation changes state from a => b',
@@ -2775,8 +2772,7 @@ describe('animation tests', function() {
27752772
})
27762773
class Cmp {
27772774
exp: any = false;
2778-
// TODO(issue/24571): remove '!'.
2779-
event!: AnimationEvent;
2775+
event: AnimationEvent|undefined;
27802776

27812777
callback = (event: any) => {
27822778
this.event = event;
@@ -2799,11 +2795,11 @@ describe('animation tests', function() {
27992795
player.finish();
28002796
flushMicrotasks();
28012797

2802-
expect(cmp.event.triggerName).toEqual('myAnimation123');
2803-
expect(cmp.event.phaseName).toEqual('done');
2804-
expect(cmp.event.totalTime).toEqual(999);
2805-
expect(cmp.event.fromState).toEqual('void');
2806-
expect(cmp.event.toState).toEqual('b');
2798+
expect(cmp.event?.triggerName).toEqual('myAnimation123');
2799+
expect(cmp.event?.phaseName).toEqual('done');
2800+
expect(cmp.event?.totalTime).toEqual(999);
2801+
expect(cmp.event?.fromState).toEqual('void');
2802+
expect(cmp.event?.toState).toEqual('b');
28072803
}));
28082804

28092805
it('should handle callbacks for multiple triggers running simultaneously', fakeAsync(() => {
@@ -2832,10 +2828,8 @@ describe('animation tests', function() {
28322828
class Cmp {
28332829
exp1: any = false;
28342830
exp2: any = false;
2835-
// TODO(issue/24571): remove '!'.
2836-
event1!: AnimationEvent;
2837-
// TODO(issue/24571): remove '!'.
2838-
event2!: AnimationEvent;
2831+
event1: AnimationEvent|undefined;
2832+
event2: AnimationEvent|undefined;
28392833
// tslint:disable:semicolon
28402834
callback1 = (event: any) => {
28412835
this.event1 = event;
@@ -2869,8 +2863,8 @@ describe('animation tests', function() {
28692863
expect(cmp.event2).toBeFalsy();
28702864

28712865
flushMicrotasks();
2872-
expect(cmp.event1.triggerName).toBeTruthy('ani1');
2873-
expect(cmp.event2.triggerName).toBeTruthy('ani2');
2866+
expect(cmp.event1?.triggerName).toBeTruthy('ani1');
2867+
expect(cmp.event2?.triggerName).toBeTruthy('ani2');
28742868
}));
28752869

28762870
it('should handle callbacks for multiple triggers running simultaneously on the same element',
@@ -2899,10 +2893,8 @@ describe('animation tests', function() {
28992893
class Cmp {
29002894
exp1: any = false;
29012895
exp2: any = false;
2902-
// TODO(issue/24571): remove '!'.
2903-
event1!: AnimationEvent;
2904-
// TODO(issue/24571): remove '!'.
2905-
event2!: AnimationEvent;
2896+
event1: AnimationEvent|undefined;
2897+
event2: AnimationEvent|undefined;
29062898
callback1 = (event: any) => {
29072899
this.event1 = event;
29082900
};
@@ -2934,8 +2926,8 @@ describe('animation tests', function() {
29342926
expect(cmp.event2).toBeFalsy();
29352927

29362928
flushMicrotasks();
2937-
expect(cmp.event1.triggerName).toBeTruthy('ani1');
2938-
expect(cmp.event2.triggerName).toBeTruthy('ani2');
2929+
expect(cmp.event1?.triggerName).toBeTruthy('ani1');
2930+
expect(cmp.event2?.triggerName).toBeTruthy('ani2');
29392931
}));
29402932

29412933
it('should handle a leave animation for multiple triggers even if not all triggers have their own leave transition specified',
@@ -3010,8 +3002,7 @@ describe('animation tests', function() {
30103002
[style({'opacity': '0'}), animate(1000, style({'opacity': '1'}))])])],
30113003
})
30123004
class Cmp {
3013-
// TODO(issue/24571): remove '!'.
3014-
event!: AnimationEvent;
3005+
event: AnimationEvent|undefined;
30153006

30163007
@HostBinding('@myAnimation2') exp: any = false;
30173008

@@ -3030,11 +3021,11 @@ describe('animation tests', function() {
30303021
fixture.detectChanges();
30313022
flushMicrotasks();
30323023

3033-
expect(cmp.event.triggerName).toEqual('myAnimation2');
3034-
expect(cmp.event.phaseName).toEqual('start');
3035-
expect(cmp.event.totalTime).toEqual(1000);
3036-
expect(cmp.event.fromState).toEqual('void');
3037-
expect(cmp.event.toState).toEqual('TRUE');
3024+
expect(cmp.event?.triggerName).toEqual('myAnimation2');
3025+
expect(cmp.event?.phaseName).toEqual('start');
3026+
expect(cmp.event?.totalTime).toEqual(1000);
3027+
expect(cmp.event?.fromState).toEqual('void');
3028+
expect(cmp.event?.toState).toEqual('TRUE');
30383029
}));
30393030

30403031
it('should always fire callbacks even when a transition is not detected', fakeAsync(() => {
@@ -3046,8 +3037,7 @@ describe('animation tests', function() {
30463037
animations: [trigger('myAnimation', [])]
30473038
})
30483039
class Cmp {
3049-
// TODO(issue/24571): remove '!'.
3050-
exp!: string;
3040+
exp: string|undefined;
30513041
log: any[] = [];
30523042
callback = (event: any) => this.log.push(`${event.phaseName} => ${event.toState}`);
30533043
}
@@ -3158,10 +3148,8 @@ describe('animation tests', function() {
31583148
})
31593149
class Cmp {
31603150
log: string[] = [];
3161-
// TODO(issue/24571): remove '!'.
3162-
exp1!: string;
3163-
// TODO(issue/24571): remove '!'.
3164-
exp2!: string;
3151+
exp1: string|undefined;
3152+
exp2: string|undefined;
31653153

31663154
cb(name: string, event: AnimationEvent) {
31673155
this.log.push(name);
@@ -3239,8 +3227,7 @@ describe('animation tests', function() {
32393227
class Cmp {
32403228
log: string[] = [];
32413229
events: {[name: string]: any} = {};
3242-
// TODO(issue/24571): remove '!'.
3243-
exp!: string;
3230+
exp: string|undefined;
32443231
items: any = [0, 1, 2, 3];
32453232

32463233
cb(name: string, phase: string, event: AnimationEvent) {
@@ -3546,7 +3533,7 @@ describe('animation tests', function() {
35463533
fixture.detectChanges();
35473534
resetLog();
35483535

3549-
const parent = cmp.parentElm!.nativeElement;
3536+
const parent = cmp.parentElm.nativeElement;
35503537

35513538
cmp.exp = true;
35523539
fixture.detectChanges();
@@ -3580,10 +3567,8 @@ describe('animation tests', function() {
35803567
class Cmp {
35813568
disableExp = false;
35823569
exp = '';
3583-
// TODO(issue/24571): remove '!'.
3584-
startEvent!: AnimationEvent;
3585-
// TODO(issue/24571): remove '!'.
3586-
doneEvent!: AnimationEvent;
3570+
startEvent: AnimationEvent|undefined;
3571+
doneEvent: AnimationEvent|undefined;
35873572
}
35883573

35893574
TestBed.configureTestingModule({declarations: [Cmp]});
@@ -3599,17 +3584,17 @@ describe('animation tests', function() {
35993584
cmp.exp = '1';
36003585
fixture.detectChanges();
36013586
flushMicrotasks();
3602-
expect(cmp.startEvent.totalTime).toEqual(9876);
3603-
expect(cmp.startEvent.disabled).toBeTruthy();
3604-
expect(cmp.doneEvent.totalTime).toEqual(9876);
3605-
expect(cmp.doneEvent.disabled).toBeTruthy();
3587+
expect(cmp.startEvent?.totalTime).toEqual(9876);
3588+
expect(cmp.startEvent?.disabled).toBeTruthy();
3589+
expect(cmp.doneEvent?.totalTime).toEqual(9876);
3590+
expect(cmp.doneEvent?.disabled).toBeTruthy();
36063591

36073592
cmp.exp = '2';
36083593
cmp.disableExp = false;
36093594
fixture.detectChanges();
36103595
flushMicrotasks();
3611-
expect(cmp.startEvent.totalTime).toEqual(9876);
3612-
expect(cmp.startEvent.disabled).toBeFalsy();
3596+
expect(cmp.startEvent?.totalTime).toEqual(9876);
3597+
expect(cmp.startEvent?.disabled).toBeFalsy();
36133598
// the done event isn't fired because it's an actual animation
36143599
}));
36153600

0 commit comments

Comments
 (0)