Skip to content

Commit 325415e

Browse files
authored
Fix useObservable edge case (#220)
* changes caused by updated prettier config * change "share()" to "shareReplay(1)"
1 parent 51baa22 commit 325415e

File tree

2 files changed

+52
-27
lines changed

2 files changed

+52
-27
lines changed

reactfire/useObservable/SuspenseSubject.ts

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
import { Observable, Subject, Subscription, Subscriber, empty } from 'rxjs';
2-
import { tap, share, catchError } from 'rxjs/operators';
1+
import { empty, Observable, Subject, Subscriber, Subscription } from 'rxjs';
2+
import { catchError, shareReplay, tap } from 'rxjs/operators';
33

44
export class SuspenseSubject<T> extends Subject<T> {
55
private _value: T | undefined;
@@ -14,9 +14,7 @@ export class SuspenseSubject<T> extends Subject<T> {
1414

1515
constructor(innerObservable: Observable<T>, private _timeoutWindow: number) {
1616
super();
17-
this._firstEmission = new Promise<void>(
18-
resolve => (this._resolveFirstEmission = resolve)
19-
);
17+
this._firstEmission = new Promise<void>(resolve => (this._resolveFirstEmission = resolve));
2018
this._innerObservable = innerObservable.pipe(
2119
tap(
2220
v => {
@@ -30,7 +28,7 @@ export class SuspenseSubject<T> extends Subject<T> {
3028
}
3129
),
3230
catchError(() => empty()),
33-
share()
31+
shareReplay(1)
3432
);
3533
// warm up the observable
3634
this._warmupSubscription = this._innerObservable.subscribe();
@@ -75,9 +73,7 @@ export class SuspenseSubject<T> extends Subject<T> {
7573
this._hasValue = false;
7674
this._value = undefined;
7775
this._error = undefined;
78-
this._firstEmission = new Promise<void>(
79-
resolve => (this._resolveFirstEmission = resolve)
80-
);
76+
this._firstEmission = new Promise<void>(resolve => (this._resolveFirstEmission = resolve));
8177
}
8278

8379
_subscribe(subscriber: Subscriber<T>): Subscription {

reactfire/useObservable/useObservable.test.tsx

Lines changed: 47 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,7 @@ describe('useObservable', () => {
3535
const observableVal = "y'all";
3636
const observable$ = new Subject<any>();
3737

38-
const { result, waitForNextUpdate } = renderHook(() =>
39-
useObservable(observable$, 'test-2', startVal)
40-
);
38+
const { result, waitForNextUpdate } = renderHook(() => useObservable(observable$, 'test-2', startVal));
4139

4240
expect(result.current).toEqual(startVal);
4341

@@ -118,9 +116,7 @@ describe('useObservable', () => {
118116
const actualComponentId = 'actual-component';
119117
const fallbackComponentId = 'fallback-component';
120118

121-
const FallbackComponent = () => (
122-
<h1 data-testid={fallbackComponentId}>Fallback</h1>
123-
);
119+
const FallbackComponent = () => <h1 data-testid={fallbackComponentId}>Fallback</h1>;
124120

125121
const Component = () => {
126122
const val = useObservable(observable$, 'test-suspense');
@@ -142,9 +138,7 @@ describe('useObservable', () => {
142138

143139
// make sure Suspense correctly renders its child after the observable emits a value
144140
expect(getByTestId(actualComponentId)).toBeInTheDocument();
145-
expect(getByTestId(actualComponentId)).toHaveTextContent(
146-
observableFinalVal
147-
);
141+
expect(getByTestId(actualComponentId)).toHaveTextContent(observableFinalVal);
148142
expect(queryByTestId(fallbackComponentId)).toBeNull();
149143
});
150144

@@ -153,9 +147,7 @@ describe('useObservable', () => {
153147
const values = ['a', 'b', 'c'];
154148
const observable$ = new Subject();
155149

156-
const { result } = renderHook(() =>
157-
useObservable(observable$, 'test-changes', startVal)
158-
);
150+
const { result } = renderHook(() => useObservable(observable$, 'test-changes', startVal));
159151

160152
expect(result.current).toEqual(startVal);
161153

@@ -182,16 +174,12 @@ describe('useObservable', () => {
182174
return (
183175
<React.Suspense fallback="loading">
184176
<ObservableConsumer data-testid={firstComponentId} />
185-
{renderSecondComponent ? (
186-
<ObservableConsumer data-testid={secondComponentId} />
187-
) : null}
177+
{renderSecondComponent ? <ObservableConsumer data-testid={secondComponentId} /> : null}
188178
</React.Suspense>
189179
);
190180
};
191181

192-
const { getByTestId, rerender } = render(
193-
<Component renderSecondComponent={false} />
194-
);
182+
const { getByTestId, rerender } = render(<Component renderSecondComponent={false} />);
195183

196184
// emit one value to the first component (second one isn't rendered yet)
197185
act(() => observable$.next(values[0]));
@@ -211,4 +199,45 @@ describe('useObservable', () => {
211199
const comp2 = await waitForElement(() => getByTestId(secondComponentId));
212200
expect(comp2).toHaveTextContent(values[1]);
213201
});
202+
203+
it(`emits the new observable's value if the observable is swapped out`, async () => {
204+
const obs1$ = new Subject();
205+
const obs2$ = new Subject();
206+
207+
let currentObs$ = obs1$;
208+
let currentObsId = 'observable-1';
209+
210+
const ObservableConsumer = props => {
211+
const val = useObservable(currentObs$, currentObsId);
212+
213+
return <h1 {...props}>{val}</h1>;
214+
};
215+
216+
const Component = () => {
217+
return (
218+
<React.Suspense fallback={<span data-testid="fallback">Loading...</span>}>
219+
<ObservableConsumer data-testid={'consumer'} />
220+
</React.Suspense>
221+
);
222+
};
223+
224+
const { getByTestId, rerender } = render(<Component />);
225+
226+
act(() => obs1$.next('Jeff'));
227+
const comp = await waitForElement(() => getByTestId('consumer'));
228+
expect(comp).toBeInTheDocument();
229+
230+
currentObs$ = obs2$;
231+
currentObsId = 'observable-2';
232+
233+
rerender(<Component />);
234+
expect(getByTestId('fallback')).toBeInTheDocument();
235+
236+
act(() => obs2$.next('James'));
237+
const refreshedComp = await waitForElement(() => getByTestId('consumer'));
238+
expect(refreshedComp).toBeInTheDocument();
239+
240+
// if useObservable doesn't re-emit, the value here will still be "Jeff"
241+
expect(refreshedComp).toHaveTextContent('James');
242+
});
214243
});

0 commit comments

Comments
 (0)