Skip to content

Commit

Permalink
fix: don't reassign closed-over parameter in fromFetch (ReactiveX#5234)
Browse files Browse the repository at this point in the history
* test: add failing test for ReactiveX#5233

* fix: don't reassign closed-over parameter in fromFetch

Closes ReactiveX#5233

* chore: add type annotation for variable
  • Loading branch information
cartant authored and Martin Sikora committed Feb 15, 2020
1 parent 747998e commit 7a1c0fa
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 3 deletions.
19 changes: 19 additions & 0 deletions spec/observables/dom/fetch-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,25 @@ describe('fromFetch', () => {
expect(mockFetch.calls[0].init.signal.aborted).to.be.true;
});

it('should not immediately abort repeat subscribers', () => {
const fetch$ = fromFetch('/foo');
expect(mockFetch.calls.length).to.equal(0);
expect(MockAbortController.created).to.equal(0);
let subscription = fetch$.subscribe();
expect(MockAbortController.created).to.equal(1);
expect(mockFetch.calls[0].init.signal.aborted).to.be.false;

subscription.unsubscribe();
expect(mockFetch.calls[0].init.signal.aborted).to.be.true;

subscription = fetch$.subscribe();
expect(MockAbortController.created).to.equal(2);
expect(mockFetch.calls[1].init.signal.aborted).to.be.false;

subscription.unsubscribe();
expect(mockFetch.calls[1].init.signal.aborted).to.be.true;
});

it('should allow passing of init object', done => {
const fetch$ = fromFetch('/foo', {method: 'HEAD'});
fetch$.subscribe({
Expand Down
9 changes: 6 additions & 3 deletions src/internal/observable/dom/fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ export function fromFetch(input: string | Request, init?: RequestInit): Observab
let abortable = true;
let unsubscribed = false;

let perSubscriberInit: RequestInit;
if (init) {
// If a signal is provided, just have it teardown. It's a cancellation token, basically.
if (init.signal) {
Expand All @@ -72,12 +73,14 @@ export function fromFetch(input: string | Request, init?: RequestInit): Observab
init.signal.addEventListener('abort', outerSignalHandler);
}
}
init = { ...init, signal };
// init cannot be mutated or reassigned as it's closed over by the
// subscriber callback and is shared between subscribers.
perSubscriberInit = { ...init, signal };
} else {
init = { signal };
perSubscriberInit = { signal };
}

fetch(input, init).then(response => {
fetch(input, perSubscriberInit).then(response => {
abortable = false;
subscriber.next(response);
subscriber.complete();
Expand Down

0 comments on commit 7a1c0fa

Please sign in to comment.