Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit fa0fdfa

Browse filesBrowse files
hoebbelsBhoebbelsB
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
Copy full SHA for fa0fdfa

File tree

2 files changed

+115
-99
lines changed
Filter options

2 files changed

+115
-99
lines changed

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

Copy file name to clipboardExpand all lines: libs/state/actions/src/lib/actions.factory.spec.ts
+103-87Lines 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

Copy file name to clipboardExpand all lines: libs/state/actions/src/lib/actions.factory.ts
+12-12Lines 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)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.