8000 Merge pull request #1829 from michaelbe812/f/use-destroy-ref-within-r… · rx-angular/rx-angular@fa0fdfa · GitHub
[go: up one dir, main page]

Skip to content

Commit fa0fdfa

Browse files
authored
Merge pull request #1829 from michaelbe812/f/use-destroy-ref-within-rx-action-factory
refactor(state): use DestroyRef instead of OnDestroy life cycle hook
2 parents 70a590d + 6005533 commit fa0fdfa

File tree

2 files changed

+115
-99
lines changed

2 files changed

+115
-99
lines changed

libs/state/actions/src/lib/actions.factory.spec.ts

Lines changed: 103 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -29,121 +29,137 @@ describe('RxActionFactory', () => {
2929
beforeAll(() => mockConsole());
3030

3131
it('should get created properly', () => {
32-
const actions = new RxActionFactory<{ prop: string }>().create();
33-
expect(typeof actions.prop).toBe('function');
34-
expect(isObservable(actions.prop)).toBeFalsy();
35-
expect(isObservable(actions.prop$)).toBeTruthy();
32+
TestBed.runInInjectionContext(() => {
33+
const actions = new RxActionFactory<{ prop: string }>().create();
34+
expect(typeof actions.prop).toBe('function');
35+
expect(isObservable(actions.prop)).toBeFalsy();
36+
expect(isObservable(actions.prop$)).toBeTruthy();
37+
});
3638
});
3739

3840
it('should emit on the subscribed channels', (done) => {
39-
const values = 'foo';
40-
const actions = new RxActionFactory<{ prop: string }>().create();
41-
const exp = values;
42-
actions.prop$.subscribe((result) => {
43-
expect(result).toBe(exp);
44-
done();
41+
TestBed.runInInjectionContext(() => {
42+
const values = 'foo';
43+
const actions = new RxActionFactory<{ prop: string }>().create();
44+
const exp = values;
45+
actions.prop$.subscribe((result) => {
46+
expect(result).toBe(exp);
47+
done();
48+
});
49+
actions.prop(values);
4550
});
46-
actions.prop(values);
4751
});
4852

4953
it('should maintain channels per create call', (done) => {
50-
const values = 'foo';
51-
const nextSpy = jest.spyOn({ nextSpy: (_: string) => void 0 }, 'nextSpy');
52-
const actions = new RxActionFactory<{ prop: string }>().create();
53-
const actions2 = new RxActionFactory<{ prop: string }>().create();
54-
const exp = values;
55-
56-
actions2.prop$.subscribe(nextSpy as unknown as (_: string) => void);
57-
actions.prop$.subscribe((result) => {
58-
expect(result).toBe(exp);
59-
done();
54+
TestBed.runInInjectionContext(() => {
55+
const values = 'foo';
56+
const nextSpy = jest.spyOn({ nextSpy: (_: string) => void 0 }, 'nextSpy');
57+
const actions = new RxActionFactory<{ prop: string }>().create();
58+
const actions2 = new RxActionFactory<{ prop: string }>().create();
59+
const exp = values;
60+
61+
actions2.prop$.subscribe(nextSpy as unknown as (_: string) => void);
62+
actions.prop$.subscribe((result) => {
63+
expect(result).toBe(exp);
64+
done();
65+
});
66+
expect(nextSpy).not.toHaveBeenCalled();
67+
actions.prop(values);
6068
});
61-
expect(nextSpy).not.toHaveBeenCalled();
62-
actions.prop(values);
6369
});
6470

6571
it('should emit and transform on the subscribed channels', (done) => {
66-
const actions = new RxActionFactory<{ prop: string }>().create({
67-
prop: () => 'transformed',
72+
TestBed.runInInjectionContext(() => {
73+
const actions = new RxActionFactory<{ prop: string }>().create({
74+
prop: () => 'transformed',
75+
});
76+
const exp = 'transformed';
77+
actions.prop$.subscribe((result) => {
78+
expect(result).toBe(exp);
79+
done();
80+
});
81+
actions.prop();
6882
});
69-
const exp = 'transformed';
70-
actions.prop$.subscribe((result) => {
71-
expect(result).toBe(exp);
72-
done();
73-
});
74-
actions.prop();
7583
});
7684

7785
it('should emit on multiple subscribed channels', (done) => {
78-
const value1 = 'foo';
79-
const value2 = 'bar';
80-
const actions = new RxActionFactory<{
81-
prop1: string;
82-
prop2: string;
83-
}>().create();
84-
const res = {};
85-
actions.prop1$.subscribe((result) => {
86-
res['prop1'] = result;
87-
});
88-
actions.prop2$.subscribe((result) => {
89-
res['prop2'] = result;
86+
TestBed.runInInjectionContext(() => {
87+
const value1 = 'foo';
88+
const value2 = 'bar';
89+
const actions = new RxActionFactory<{
90+
prop1: string;
91+
prop2: string;
92+
}>().create();
93+
const res = {};
94+
actions.prop1$.subscribe((result) => {
95+
res['prop1'] = result;
96+
});
97+
actions.prop2$.subscribe((result) => {
98+
res['prop2'] = result;
99+
});
100+
actions({ prop1: value1, prop2: value2 });
101+
expect(res).toStrictEqual({ prop1: value1, prop2: value2 });
102+
done();
90103
});
91-
actions({ prop1: value1, prop2: value2 });
92-
expect(res).toStrictEqual({ prop1: value1, prop2: value2 });
93-
done();
94104
});
95105

96106
it('should emit on multiple subscribed channels over mreged output', (done) => {
97-
const value1 = 'foo';
98-
const value2 = 'bar';
99-
const actions = new RxActionFactory<{
100-
prop1: string;
101-
prop2: string;
102-
}>().create();
103-
104-
const res = [];
105-
expect(typeof actions.$).toBe('function');
106-
actions.$(['prop1', 'prop2']).subscribe((result) => {
107-
res.push(result);
107+
TestBed.runInInjectionContext(() => {
108+
const value1 = 'foo';
109+
const value2 = 'bar';
110+
const actions = new RxActionFactory<{
111+
prop1: string;
112+
prop2: string;
113+
}>().create();
114+
115+
const res = [];
116+
expect(typeof actions.$).toBe('function');
117+
actions.$(['prop1', 'prop2']).subscribe((result) => {
118+
res.push(result);
119+
});
120+
actions({ prop1: value1, prop2: value2 });
121+
expect(res.length).toBe(2);
122+
expect(res).toStrictEqual([value1, value2]);
123+
done();
108124
});
109-
actions({ prop1: value1, prop2: value2 });
110-
expect(res.length).toBe(2);
111-
expect(res).toStrictEqual([value1, value2]);
112-
done();
113125
});
114126

115127
it('should destroy all created actions', (done) => {
116-
let numCalls = 0;
117-
let numCalls2 = 0;
118-
const factory = new RxActionFactory<{ prop: void }>();
119-
const actions = factory.create();
120-
const actions2 = factory.create();
121-
122-
actions.prop$.subscribe(() => ++numCalls);
123-
actions2.prop$.subscribe(() => ++numCalls2);
124-
expect(numCalls).toBe(0);
125-
expect(numCalls2).toBe(0);
126-
actions.prop();
127-
actions2.prop();
128-
expect(numCalls).toBe(1);
129-
expect(numCalls2).toBe(1);
130-
factory.destroy();
131-
actions.prop();
132-
actions2.prop();
133-
expect(numCalls).toBe(1);
134-
expect(numCalls2).toBe(1);
135-
done();
128+
TestBed.runInInjectionContext(() => {
129+
let numCalls = 0;
130+
let numCalls2 = 0;
131+
const factory = new RxActionFactory<{ prop: void }>();
132+
const actions = factory.create();
133+
const actions2 = factory.create();
134+
135+
actions.prop$.subscribe(() => ++numCalls);
136+
actions2.prop$.subscribe(() => ++numCalls2);
137+
expect(numCalls).toBe(0);
138+
expect(numCalls2).toBe(0);
139+
actions.prop();
140+
actions2.prop();
141+
expect(numCalls).toBe(1);
142+
expect(numCalls2).toBe(1);
143+
factory.destroy();
144+
actions.prop();
145+
actions2.prop();
146+
expect(numCalls).toBe(1);
147+
expect(numCalls2).toBe(1);
148+
done();
149+
});
136150
});
137151

138152
it('should throw if a setter is used', (done) => {
139-
const factory = new RxActionFactory<{ prop: number }>();
140-
const actions = factory.create();
153+
TestBed.runInInjectionContext(() => {
154+
const factory = new RxActionFactory<{ prop: number }>();
155+
const actions = factory.create();
141156

142-
expect(() => {
143-
(actions as any).prop = 0;
144-
}).toThrow('');
157+
expect(() => {
158+
(actions as any).prop = 0;
159+
}).toThrow('');
145160

146-
done();
161+
done();
162+
});
147163
});
148164

149165
test('should isolate errors and invoke provided ', async () => {

libs/state/actions/src/lib/actions.factory.ts

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,10 @@
1-
import { ErrorHandler, Injectable, OnDestroy, Optional } from '@angular/core';
1+
import {
2+
DestroyRef,
3+
ErrorHandler,
4+
inject,
5+
Injectable,
6+
Optional,
7+
} from '@angular/core';
28
import { Subject } from 'rxjs';
39
import { actionProxyHandler } from './proxy';
410
import { Actions, ActionTransforms, EffectMap, RxActions } from './types';
@@ -22,10 +28,12 @@ type SubjectMap<T> = { [K in keyof T]: Subject<T[K]> };
2228
* }
2329
*/
2430
@Injectable()
25-
export class RxActionFactory<T extends Partial<Actions>> implements OnDestroy {
26-
private subjects: SubjectMap<T>[] = [] as SubjectMap<T>[];
31+
export class RxActionFactory<T extends Partial<Actions>> {
32+
private readonly subjects: SubjectMap<T>[] = [] as SubjectMap<T>[];
2733

28-
constructor(@Optional() private readonly errorHandler?: ErrorHandler) {}
34+
constructor(@Optional() private readonly errorHandler?: ErrorHandler) {
35+
inject(DestroyRef).onDestroy(() => this.destroy());
36+
}
2937

3038
/*
3139
* Returns a object based off of the provided typing with a separate setter `[prop](value: T[K]): void` and observable stream `[prop]$: Observable<T[K]>`;
@@ -96,12 +104,4 @@ export class RxActionFactory<T extends Partial<Actions>> implements OnDestroy {
96104
Object.values(s).forEach((subject: any) => subject.complete());
97105
});
98106
}
99-
100-
/**
101-
* @internal
102-
* Internally used to clean up potential subscriptions to the subjects. (For Actions it is most probably a rare case but still important to care about)
103-
*/
104-
ngOnDestroy() {
105-
this.destroy();
106-
}
107107
}

0 commit comments

Comments
 (0)
0