Skip to content

Commit 234c6ee

Browse files
edbznedbzn
authored
Merge pull request rx-angular#1369 from rx-angular/test/make-rx-for-stable
feat(template): make rxFor stable
2 parents 52904fb + 21f2bea commit 234c6ee

File tree

28 files changed

+1752
-102
lines changed

28 files changed

+1752
-102
lines changed

‎apps/demos/src/app/features/template/rx-context/rx-context.module.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { NgModule } from '@angular/core';
2-
import { ForModule } from '@rx-angular/template/experimental/for';
2+
import { ForModule } from '@rx-angular/template/for';
33
import { LetModule } from '@rx-angular/template/let';
44
import { VisualizerModule } from '../../../shared/debug-helper/visualizer';
55
import { StrategySelectModule } from '../../../shared/debug-helper/strategy-select';

‎apps/demos/src/app/features/template/rx-for/error-handling/rx-for-error-handling.module.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { NgModule } from '@angular/core';
22
import { CommonModule } from '@angular/common';
33
import { RouterModule } from '@angular/router';
4-
import { ForModule } from '@rx-angular/template/experimental/for';
4+
import { ForModule } from '@rx-angular/template/for';
55
import { StrategySelectModule } from '../../../../shared/debug-helper/strategy-select/strategy-select.module';
66
import { ValueProvidersModule } from '../../../../shared/debug-helper/value-provider/value-providers.module';
77
import { ErrorHandlingParentComponent } from './error-handling-parent.component';

‎apps/demos/src/app/features/template/rx-for/list-actions/list-actions.component.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,22 @@ import {
22
AfterViewInit,
33
ChangeDetectorRef,
44
Component,
5-
ElementRef, NgZone,
5+
ElementRef,
6+
NgZone,
67
QueryList,
78
ViewChild,
89
ViewChildren,
9-
ViewEncapsulation
10+
ViewEncapsulation,
1011
} from '@angular/core';
1112
import { asyncScheduler } from '@rx-angular/cdk/zone-less/rxjs';
12-
import { BehaviorSubject, defer, merge, scheduled, Subject } from 'rxjs';
13+
import {
14+
BehaviorSubject,
15+
defer,
16+
merge,
17+
Observable,
18+
scheduled,
19+
Subject,
20+
} from 'rxjs';
1321
import { environment } from '../../../../../environments/environment';
1422
import {
1523
ArrayProviderService,

‎apps/demos/src/app/features/template/rx-for/list-actions/list-actions.module.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import { MatIconModule } from '@angular/material/icon';
88
import { MatInputModule } from '@angular/material/input';
99
import { RouterModule } from '@angular/router';
1010
import { LetModule, PushModule } from '@rx-angular/template';
11-
import { ForModule } from '@rx-angular/template/experimental/for';
11+
import { ForModule } from '@rx-angular/template/for';
1212
import { UnpatchModule } from '@rx-angular/template/unpatch';
1313
import { DirtyChecksModule } from '../../../../shared/debug-helper/dirty-checks';
1414
import { ValueProvidersModule } from '../../../../shared/debug-helper/value-provider/value-providers.module';

‎apps/demos/src/app/features/template/rx-for/nested-lists/nested-lists.module.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { NgModule } from '@angular/core';
22
import { CommonModule } from '@angular/common';
3-
import { ForModule } from '@rx-angular/template/experimental/for';
3+
import { ForModule } from '@rx-angular/template/for';
44
import { LetModule } from '@rx-angular/template/let';
55
import { PushModule } from '@rx-angular/template/push';
66
import { VisualizerModule } from '../../../../shared/debug-helper/visualizer';

‎apps/demos/src/app/features/template/rx-for/route-change/route-change.module.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { MatButtonModule } from '@angular/material/button';
55
import { MatIconModule } from '@angular/material/icon';
66
import { MatTabsModule } from '@angular/material/tabs';
77
import { RouterModule, Routes } from '@angular/router';
8-
import { ForModule } from '@rx-angular/template/experimental/for';
8+
import { ForModule } from '@rx-angular/template/for';
99
import { LetModule } from '@rx-angular/template/let';
1010
import { RouteChangeComponent } from './route-change.component';
1111
import { RoutedNgForComponent } from './routed-ng-for.component';

‎apps/ssr-e2e/src/integration/app.spec.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ describe('@rx-angular/template universal rendering', () => {
1111
});
1212

1313
describe('LetDirective', () => {
14-
it('should display green text using', () => {
14+
it('should display green text', () => {
1515
cy.request('http://localhost:4200').its('body').then(
1616
html => {
1717
const el = Cypress.$(html).find('#let');
@@ -20,4 +20,17 @@ describe('@rx-angular/template universal rendering', () => {
2020
)
2121
});
2222
});
23+
24+
describe('RxFor', () => {
25+
it('should display green and purple text', () => {
26+
cy.request('http://localhost:4200').its('body').then(
27+
html => {
28+
const el = Cypress.$(html).find('.for');
29+
expect(el.length).eq(2);
30+
expect(el.eq(0)).to.include.text('green');
31+
expect(el.eq(1)).to.include.text('purple');
32+
}
33+
)
34+
});
35+
});
2336
});

‎apps/ssr/src/app/app.component.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,19 @@ import { BehaviorSubject } from 'rxjs';
88
<div id="let" *rxLet="color$ as color; strategy">{{ color }}</div>
99
<div id="push">{{ color$ | push }}</div>
1010
<div id="unpatch" [unpatch]="['click']" (click)="onClick()"></div>
11+
<div class="for" *rxFor="let color of colors$">{{ color }}</div>
1112
`,
1213
})
1314
export class AppComponent implements OnInit {
1415
color$ = new BehaviorSubject('red');
16+
colors$ = new BehaviorSubject(['red']);
1517

1618
constructor(@Inject(PLATFORM_ID) private platformId: string) {}
1719

1820
ngOnInit() {
1921
if (isPlatformServer(this.platformId)) {
2022
this.color$.next('green');
23+
this.colors$.next(['green', 'purple']);
2124
}
2225
}
2326

‎apps/ssr/src/app/app.module.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
import { BrowserModule } from '@angular/platform-browser';
21
import { NgModule } from '@angular/core';
2+
import { BrowserModule } from '@angular/platform-browser';
33
import { PushModule } from '@rx-angular/template';
4+
import { ForModule } from '@rx-angular/template/for';
45
import { LetModule } from '@rx-angular/template/let';
56
import { UnpatchModule } from '@rx-angular/template/unpatch';
6-
77
import { AppComponent } from './app.component';
88

99
@NgModule({
@@ -13,6 +13,7 @@ import { AppComponent } from './app.component';
1313
PushModule,
1414
LetModule,
1515
UnpatchModule,
16+
ForModule,
1617
],
1718
providers: [],
1819
bootstrap: [AppComponent],

‎libs/cdk/template/src/lib/list-template-manager.ts

Lines changed: 66 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
import {
22
EmbeddedViewRef,
3+
IterableChanges,
34
IterableDiffer,
45
IterableDiffers,
56
NgIterable,
67
TemplateRef,
78
TrackByFunction,
89
} from '@angular/core';
9-
import { combineLatest, Observable, of, OperatorFunction } from 'rxjs';
10+
import { combineLatest, MonoTypeOperatorFunction, Observable, of } from 'rxjs';
1011
import {
1112
catchError,
1213
distinctUntilChanged,
@@ -36,7 +37,7 @@ import { notifyAllParentsIfNeeded } from './utils';
3637
export interface RxListManager<T> {
3738
nextStrategy: (config: string | Observable<string>) => void;
3839

39-
render(changes$: Observable<NgIterable<T>>): Observable<void>;
40+
render(changes$: Observable<NgIterable<T>>): Observable<NgIterable<T> | null>;
4041
}
4142

4243
export function createListTemplateManager<
@@ -50,7 +51,6 @@ export function createListTemplateManager<
5051
> & {
5152
templateRef: TemplateRef<C>;
5253
};
53-
//
5454
trackBy: TrackByFunction<T>;
5555
iterableDiffers: IterableDiffers;
5656
}): RxListManager<T> {
@@ -65,7 +65,16 @@ export function createListTemplateManager<
6565
const errorHandler = createErrorHandler(renderSettings.errorHandler);
6666
const ngZone = patchZone ? patchZone : undefined;
6767
const strategyHandling$ = strategyHandling(defaultStrategyName, strategies);
68-
const differ: IterableDiffer<T> = iterableDiffers.find([]).create(trackBy);
68+
69+
let _differ: IterableDiffer<T> | undefined;
70+
function getDiffer(values: NgIterable<T>): IterableDiffer<T> | null {
71+
if (_differ) {
72+
return _differ;
73+
}
74+
return values
75+
? (_differ = iterableDiffers.find(values).create(trackBy))
76+
: null;
77+
}
6978
// type, context
7079
/* TODO (regarding createView): this is currently not in use. for the list-manager this would mean to provide
7180
functions for not only create. developers than should have to provide create, move, remove,... the whole thing.
@@ -84,38 +93,58 @@ export function createListTemplateManager<
8493
nextStrategy(nextConfig: Observable<string>): void {
8594
strategyHandling$.next(nextConfig);
8695
},
87-
render(values$: Observable<NgIterable<T>>): Observable<any> {
96+
render(
97+
values$: Observable<NgIterable<T>>
98+
): Observable<NgIterable<T> | null> {
8899
return values$.pipe(render());
89100
},
90101
};
91102

92-
function render(): OperatorFunction<NgIterable<T>, any> {
93-
return (o$: Observable<NgIterable<T>>): Observable<any> =>
103+
function handleError() {
104+
return (o$) =>
105+
o$.pipe(
106+
catchError((err: Error) => {
107+
partiallyFinished = false;
108+
errorHandler.handleError(err);
109+
return of(null);
110+
})
111+
);
112+
}
113+
114+
function render(): MonoTypeOperatorFunction<NgIterable<T> | null> {
115+
return (o$: Observable<NgIterable<T>>): Observable<NgIterable<T> | null> =>
94116
combineLatest([
95117
o$,
96118
strategyHandling$.strategy$.pipe(distinctUntilChanged()),
97119
]).pipe(
98-
// map iterable to latest diff
99120
map(([iterable, strategy]) => {
100-
if (partiallyFinished) {
101-
const currentIterable = [];
102-
for (let i = 0, ilen = viewContainerRef.length; i < ilen; i++) {
103-
const viewRef = <EmbeddedViewRef<C>>viewContainerRef.get(i);
104-
currentIterable[i] = viewRef.context.$implicit;
121+
const differ = getDiffer(iterable);
122+
let changes: IterableChanges<T>;
123+
if (differ) {
124+
if (partiallyFinished) {
125+
const currentIterable = [];
126+
for (let i = 0, ilen = viewContainerRef.length; i < ilen; i++) {
127+
const viewRef = <EmbeddedViewRef<C>>viewContainerRef.get(i);
128+
currentIterable[i] = viewRef.context.$implicit;
129+
}
130+
differ.diff(currentIterable);
105131
}
106-
differ.diff(currentIterable);
132+
changes = differ.diff(iterable);
107133
}
108134
return {
109-
changes: differ.diff(iterable),
110-
items: iterable != null && Array.isArray(iterable) ? iterable : [],
135+
changes,
136+
iterable,
111137
strategy,
112138
};
113139
}),
114140
// Cancel old renders
115-
switchMap(({ changes, items, strategy }) => {
141+
switchMap(({ changes, iterable, strategy }) => {
116142
if (!changes) {
117143
return of([]);
118144
}
145+
const values = iterable || [];
146+
// TODO: we might want to treat other iterables in a more performant way than Array.from()
147+
const items = Array.isArray(values) ? values : Array.from(iterable);
119148
const listChanges = listViewHandler.getListChanges(changes, items);
120149
changesArr = listChanges[0];
121150
const insertedOrRemoved = listChanges[1];
@@ -126,33 +155,21 @@ export function createListTemplateManager<
126155
);
127156
partiallyFinished = true;
128157
notifyParent = insertedOrRemoved && parent;
129-
return new Observable((subscriber) => {
130-
const s = combineLatest(
131-
// emit after all changes are rendered
132-
applyChanges$.length > 0 ? applyChanges$ : [of(items)]
133-
)
134-
.pipe(
135-
tap(() => (partiallyFinished = false)),
136-
// somehow this makes the strategySelect work
137-
notifyAllParentsIfNeeded(
138-
injectingViewCdRef,
139-
strategy,
140-
() => notifyParent,
141-
ngZone
142-
),
143-
map(() => items),
144-
catchError((e) => {
145-
partiallyFinished = false;
146-
errorHandler.handleError(e);
147-
return of(items);
148-
})
149-
)
150-
.subscribe(subscriber);
151-
return () => {
152-
s.unsubscribe();
153-
};
154-
});
155-
})
158+
return combineLatest(
159+
applyChanges$.length > 0 ? applyChanges$ : [of(null)]
160+
).pipe(
161+
tap(() => (partiallyFinished = false)),
162+
notifyAllParentsIfNeeded(
163+
injectingViewCdRef,
164+
strategy,
165+
() => notifyParent,
166+
ngZone
167+
),
168+
handleError(),
169+
map(() => iterable)
170+
);
171+
}),
172+
handleError()
156173
);
157174
}
158175

@@ -198,7 +215,11 @@ export function createListTemplateManager<
198215
listViewHandler.updateView(payload[0], payload[1], count);
199216
break;
200217
case RxListTemplateChangeType.context:
201-
listViewHandler.updateUnchangedContext(payload[1], count);
218+
listViewHandler.updateUnchangedContext(
219+
payload[0],
220+
payload[1],
221+
count
222+
);
202223
break;
203224
}
204225
},

0 commit comments

Comments
 (0)