Reputation: 2308
I have been trying to understand the right way to use a for loop for an array of promises. In my code, my array has 3 elements, but it only has data the first time through the loop.
private populateRequest(connection: Connection, myArray: any[], containerId: string): Promise<void> {
// tslint:disable-next-line:no-shadowed-variable
return new Promise(async (resolve, reject) => {
const promises: Array<Promise<SomeType>> = [];
let status = '';
// tslint:disable-next-line:prefer-for-of
for (let i = 0; i < myArray.length; i++) {
const data = await this.getResolvedPromise(myArray[i])
.then(response => response)
.catch(err => console.log(err));
if (this.flags.hasOwnProperty('prop1')) {
status = 'Active';
} else if (this.flags.hasOwnProperty('prop2')) {
status = 'Inactive';
} else if (data[0]['WidgetProp1'] === 'Active') {
status = 'Inactive';
} else if (data[0]['WidgetProp1'] === 'Inactive') {
status = 'Active';
}
const myMetaObj = {
prop3: data[0]['WidgetProp3'],
status
};
const myMetaMember = {
ContainerId: containerId,
ContentEntityId: data[0]['WidgetId'],
Body: data[0]['WidgetBody'],
Metadata: myMetaObj
};
promises.push(myMetaMember);
}
Promise.all(promises)
.then(() => resolve())
.catch(err => reject(err));
});
}
private getResolvedPromise(target: Promise<any>): Promise<any> {
// tslint:disable-next-line:no-any
return new Promise((resolve, reject) => {
// tslint:disable-next-line:no-any
Promise.all([target])
.then(() => resolve(target))
.catch(err => reject(err));
});
}
The push works as intended the first time, but not subsequently.
I understand this is because of async code and calls not finishing, but am not sure why my Promise.all()
doesn't work correctly.
Halps?
Upvotes: 0
Views: 244
Reputation: 707258
There are a couple things wrong here.
First off, you aren't pushing promises into your promises
variable. You're pushing myMetaMember
variables into that array.
Second, if you're already serializing your asynchronous requests with await
, you don't need to use Promise.all()
at all.
Third, you're using several anti-patterns by wrapping existing promises in additional layers of manually created promises when you do not need to do that.
Fourth, you generally don't want to mix await
and .then()
and .catch()
. To catch errors from await
, use try/catch
. There is no need for .then()
because await
already gets the value for you.
Fifth, getResolvePromise()
does not appear to do anything useful. It calls Promise.all([target])
and then returns a promise that resolves to target
. That doesn't appear to be accomplishing anything useful. What did you think that was accomplishing.
Since you've now clarified that myArray
is an array of promises that each result to an object, here's my best interpretation of what you're trying to do:
private populateRequest(connection: Connection, myArray: any[], containerId: string): Promise<Array<{ContainerId: string; ContentEntityId: unknown; Body: unknown; Metadata: {prop3: unknown; status: string}}>> {
return Promise.all(myArray).then(results => {
return results.map(data => {
let status = '';
if (this.flags.hasOwnProperty('prop1')) {
status = 'Active';
} else if (this.flags.hasOwnProperty('prop2')) {
status = 'Inactive';
} else if (data[0]['WidgetProp1'] === 'Active') {
status = 'Inactive';
} else if (data[0]['WidgetProp1'] === 'Inactive') {
status = 'Active';
}
const myMetaObj = {
prop3: data[0]['WidgetProp3'],
status
};
const myMetaMember = {
ContainerId: containerId,
ContentEntityId: data[0]['WidgetId'],
Body: data[0]['WidgetBody'],
Metadata: myMetaObj
};
return myMetaMember;
});
});
}
To get the results from myArray
(which you said was an array or promises that each resolve to an object, you would use Promise.all(myArray)
. That will return a single promise that resolves to an array of results which you can then use .then()
to get the array of results. You can then iterate that array to build your new objects based on their data.
P.S. If myArray
is really an array of promises, then you shouldn't be declaring it as myArray: any[]
. That defeats part of the reason for TypeScript in that it doesn't teach the reader (me) what it is and it doesn't let TypeScript enforce that it's being passed the right thing.
Upvotes: 2
Reputation: 162
i cant really make sense of your example. but what i see is that you are using await
which syncronously resolves the promise from getResolvedPromise
. You are also using .then
in conjunction with await which doesn't make sense as it does the same thing, but just in another style as 'await'. So either you do:
const data = await promise;
// data is resolved here
or
promise.then(
data => // data is resolved here
)
If you have to iterate over a structure with a for loop instead of using .map
i would suggest to do something like this:
async someFunction(): Promise<any[]> {
promises: Promise<any>[] = []
for(const value of list) {
promises.push(new Promise((resolve, reject) => {
const data = await something
// do something with data
const fancyObject = {}
resolve(fancyObject);
}));
return Promise.all(promises);
}
You see that the logic which is depending on the data object only runs after the promise is resolved via the await
operator. So this logic needs to be wrapped in a Promise itself.
Promise.all
in it self returns a Promise so you dont need to wrap it in another Promise and resolves to an array containing all your fancyObjects.
Beware also that await syntax needs a try catch block in order to catch errors happening while the promise resolves.
I hope i understood you correctly and you can make something of my example.
Upvotes: 0