Skip to content

Commit

Permalink
Fix references to uninitialized Promise variables.
Browse files Browse the repository at this point in the history
The following code should cause the promise to be rejected, because the
`promise` variable will not be initialized until after the `new
Promise(...)` expression finishes evaluating, which happens after the
callback returns:

  const promise = new Promise((resolve, reject) => {
    console.log(promise);
  });

TypeScript compiles this code down to `var promise = ...` instead of
`const promise = ...`, which is probably why the `ReferenceError` is not
fatal, but the `promise` variable is definitely undefined either way.

It's a shame that TypeScript thinks this code is perfectly fine, though
temporal dead zone errors are admittedly difficult to simulate
(efficiently, at least) using `var` declarations.

Good thing the `promise` property of `QueryPromise` objects is never used!
  • Loading branch information
benjamn committed May 1, 2018
1 parent c6faf84 commit d68046f
Showing 1 changed file with 6 additions and 11 deletions.
17 changes: 6 additions & 11 deletions packages/apollo-client/src/core/QueryManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ const defaultQueryInfo = {
};

export interface QueryPromise {
promise: Promise<ApolloQueryResult<any>>;
resolve: (result: ApolloQueryResult<any>) => void;
reject: (error: Error) => void;
}
Expand Down Expand Up @@ -681,8 +680,9 @@ export class QueryManager<TStore> {
options.notifyOnNetworkStatusChange = false;

const requestId = this.idCounter;
const resPromise = new Promise<ApolloQueryResult<T>>((resolve, reject) => {
this.addFetchQueryPromise<T>(requestId, resPromise, resolve, reject);

return new Promise<ApolloQueryResult<T>>((resolve, reject) => {
this.addFetchQueryPromise<T>(requestId, resolve, reject);

return this.watchQuery<T>(options, false)
.result()
Expand All @@ -695,8 +695,6 @@ export class QueryManager<TStore> {
reject(error);
});
});

return resPromise;
}

public generateQueryId() {
Expand Down Expand Up @@ -751,12 +749,10 @@ export class QueryManager<TStore> {
// Adds a promise to this.fetchQueryPromises for a given request ID.
public addFetchQueryPromise<T>(
requestId: number,
promise: Promise<ApolloQueryResult<T>>,
resolve: (result: ApolloQueryResult<T>) => void,
reject: (error: Error) => void,
) {
this.fetchQueryPromises.set(requestId.toString(), {
promise,
resolve,
reject,
});
Expand Down Expand Up @@ -1060,8 +1056,9 @@ export class QueryManager<TStore> {

let resultFromStore: any;
let errorsFromStore: any;
const retPromise = new Promise<ApolloQueryResult<T>>((resolve, reject) => {
this.addFetchQueryPromise<T>(requestId, retPromise, resolve, reject);

return new Promise<ApolloQueryResult<T>>((resolve, reject) => {
this.addFetchQueryPromise<T>(requestId, resolve, reject);
const subscription = execute(this.deduplicator, operation).subscribe({
next: (result: ExecutionResult) => {
// default the lastRequestId to 1
Expand Down Expand Up @@ -1154,8 +1151,6 @@ export class QueryManager<TStore> {
subscriptions: subscriptions.concat([subscription]),
}));
});

return retPromise;
}

// Refetches a query given that query's name. Refetches
Expand Down

0 comments on commit d68046f

Please sign in to comment.