Promiseで処理の実行順を保証したい

Question

前提・実現したいこと

クラスのプロパティに設定された複数の画像（base64文字列を格納した配列）を順にアップロードし、アップロードできた画像のパスをクラスのプロパティに設定し直したいです。

その後（画像がすべてアップロードされた後に）、クラスのプロパティをデータベースに保存したいのですが、その前段階でつまづいてます。
原因はわかっていますが、その解決方法がわかりません。

reduceを使っているからいけないのでしょうか。JS初心者なのでお手柔らかにご教示いただけると助かります。

発生している問題・エラーメッセージ

原因はアップロードが完了する（画像パスが設定される）前に、データベースに保存しにいっていることです。
以下画像の通り、アップロード完了時のログ（fuga）より前に、データベース保存時のログ（piyo ）出力されており、画像がundefinedのままデータを保存しにいっていることがわかります。
※以下は２ファイルをアップロードした結果です

該当のソースコード

save(data: Product): Promise<any> {
    return this.upload(data.images).then(images => {
        console.log(images); // 1. undefind
        data.images = images; // ここでプロパティを設定し直したい
        return data;
    }).then((data: Product) => {
        console.log("piyo"); // 2. piyo
        this.firestore.doc<Product>(`products/${id}`).update(data); // 3. error
    }).catch(err => {
        console.log('upload failed', err);
    });
}

upload(images: string[]): Promise<string[]> {
    return images.reduce((promise: Promise<any>, image: string) => {
        return promise.then((uploadedImages) => {
            const path = `${new Date().getTime()}`;

            this.storage.ref(path).putString(image, 'data_url').then(snapshot => {
                console.log("fuga"); // 4. fuga
                uploadedImages.push(path);
                return uploadedImages;
            });
        });
    }, Promise.resolve([]));
}

補足情報（FW/ツールのバージョンなど）

ストレージはFirebaseを使用しています。

Answer 1

return promise.then((uploadedImages) => {で始まる関数に返り値がないため、この中で生成されたPromiseは、終了を待たれず放置されることになってしまいます。

ここから書き換えてもいいのですが、「大量のPromiseを全部待たせる」Promise.allという関数があるので、それを使ったほうがいいでしょう。

upload(images: string[]): Promise<string[]> {
  return Promise.all(images.map(image => {
    const path = `${new Date().getTime()}`;

    return this.storage.ref(path).putString(image, 'data_url').then(() => path);
  }))
}

Comments

[退会済みユーザー]

> 返り値がないため、この中で生成されたPromiseは、終了を待たれず放置されることになってしまいます。

確かに返り値がありませんでしたね。
ただ返り値があったらreduceでもPromise.allのように終了を待たせることができるのでしょうか？
今回のように「複数の子処理の終了を待たせる」いという状況においてはPromise.allという選択肢しかなかったのかというところが気になりました。
理解が追いついてなくてすみません。

> 「大量のPromiseを全部待たせる」Promise.allという関数があるので、それを使ったほうがいいでしょう。

勉強になります、ありがとうございます！

[maisumakun]

はい、Promise.prototype.thenのコールバック関数がPromiseを返した場合、.thenの返り値は「コールバックが返したPromiseも待つ」という挙動になります。

https://developer.mozilla.org/ja/docs/Web/JavaScript/Reference/Global_Objects/Promise/then

なお、reduceでつないでいくと、必然的に「1つのPromiseが終わってから次のPromiseを開始する」というような挙動になります。画像アップロードを並列してやりたいのであれば、Promise.allが適切、ということになります。

[退会済みユーザー]

大変勉強になりました、ありがとうございます！

Answer 2

果たして、非同期処理にしなければならないんでしょうか。
Promise使わずに順番に実行していけばいいんじゃ

Answer 3

bluebird の Promise.map もオススメ。
並列処理する数を簡単に制限できるので色んな場面で役立ちます。
なによりネイティブ実装の Promise より処理速い。

Promise.map | bluebird

const Promise = require('bluebird');

upload(images: string[]): Promise<string[]> {
  return Promise.map(
    images,
    (image: string) => {
      const path = `${new Date().getTime()}`;

      return this.storage
        .ref(path)
        .putString(image, 'data_url')
        .then(() => path);
    },
    { concurrency: 5 } // 並列処理の上限設定
  );
}

Comments

[退会済みユーザー]

ありがとうございます！参考にさせていただきます！