Skip to content

Commit

Permalink
Tolerate undefined concast.sources if complete called earlier tha…
Browse files Browse the repository at this point in the history
…n `concast.start` (#10526)

Fixes issue #10262
  • Loading branch information
benjamn authored Feb 6, 2023
1 parent 7ff4b93 commit 1d13de4
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 6 deletions.
5 changes: 5 additions & 0 deletions .changeset/shaggy-bees-attack.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@apollo/client": patch
---

Tolerate undefined `concast.sources` if `complete` called earlier than `concast.start`
18 changes: 13 additions & 5 deletions src/utilities/observables/Concast.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,12 @@ export class Concast<T> extends Observable<T> {
}
}

// A consumable array of source observables, incrementally consumed
// each time this.handlers.complete is called.
private sources: Source<T>[];
// A consumable array of source observables, incrementally consumed each time
// this.handlers.complete is called. This private field is not initialized
// until the concast.start method is called, which can happen asynchronously
// if a Promise is passed to the Concast constructor, so undefined is a
// possible value for this.sources before concast.start is called.
private sources: Source<T>[] | undefined;

private start(sources: ConcastSourcesIterable<T>) {
if (this.sub !== void 0) return;
Expand Down Expand Up @@ -185,9 +188,14 @@ export class Concast<T> extends Observable<T> {
},

complete: () => {
const { sub } = this;
const { sub, sources = [] } = this;
if (sub !== null) {
const value = this.sources.shift();
// If complete is called before concast.start, this.sources may be
// undefined, so we use a default value of [] for sources. That works
// here because it falls into the if (!value) {...} block, which
// appropriately terminates the Concast, even if this.sources might
// eventually have been initialized to a non-empty array.
const value = sources.shift();
if (!value) {
if (sub) setTimeout(() => sub.unsubscribe());
this.sub = null;
Expand Down
32 changes: 31 additions & 1 deletion src/utilities/observables/__tests__/Concast.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { itAsync } from "../../../testing/core";
import { Observable } from "../Observable";
import { Concast } from "../Concast";
import { Concast, ConcastSourcesIterable } from "../Concast";

describe("Concast Observable (similar to Behavior Subject in RxJS)", () => {
itAsync("can concatenate other observables", (resolve, reject) => {
Expand Down Expand Up @@ -30,6 +30,36 @@ describe("Concast Observable (similar to Behavior Subject in RxJS)", () => {
});
});

itAsync("Can tolerate being completed before input Promise resolves", (resolve, reject) => {
let resolvePromise: (sources: ConcastSourcesIterable<number>) => void;
const delayPromise = new Promise<ConcastSourcesIterable<number>>(resolve => {
resolvePromise = resolve;
});

const concast = new Concast<number>(delayPromise);
const observer = {
next() {
reject(new Error("should not have called observer.next"));
},
error: reject,
complete() {
reject(new Error("should not have called observer.complete"));
},
};

concast.addObserver(observer);
concast.removeObserver(observer);

return concast.promise.then(finalResult => {
expect(finalResult).toBeUndefined();
resolvePromise([]);
return delayPromise;
}).then(delayedPromiseResult => {
expect(delayedPromiseResult).toEqual([]);
resolve();
}).catch(reject);
});

itAsync("behaves appropriately if unsubscribed before first result", (resolve, reject) => {
const concast = new Concast([
new Promise(resolve => setTimeout(resolve, 100)).then(
Expand Down

0 comments on commit 1d13de4

Please sign in to comment.