Can I use a promise in .then arguments?

Question

I have 2 functions:

the first one make an http post to get an xml string

function post(url, formData) {
    return new Promise(function (resolve, reject) {

        // make an http post and get results (xml string)
        request(url, formData, function(error, xml) {

            if (error) reject(error)

            resolve(xml)
        })
    })
}

the second transform that xml to an object

function xmlToObject(xml) {

    return new Promise( function (resolve, reject) {

        // transform xml string to an object using xml2js for example
        xml2js(xml, function(error, obj) {

            if (error) reject(error)

            resolve(obj)
        })
    })
}

Now I want to call post request and get an xml string and then transform it to an object, so which one is correct and why:

post(url, formData).then( function (xml) {

    xmlToObject(xml).then( function (obj) {
        // do some work
    })

})

Or

post(url, formData).then( function (xml) {

    xmlToObject(xml).then( function (obj) {
        return obj
    })

}).then( function (obj) {
    // do some work
})

Or

post(url, formData).then( function (xml) {

    return xmlToObject(xml)

}).then( function (obj) {
    // do some work
})

Or

post(url, formData).then( xmlToObject )
.then( function (obj) {
    // do some work
})

js is flexible. just because there's more than one way to do something doesn't mean one way is "wrong," but if you're going to use promises you might as well take advantage of the chainability, which is more readable than callbacks. i don't see any benefit in mixing the two. — I wrestled a bear once., Nov 23 '16 at 02:07
@Iwrestledabearonce. Some of these approaches *are* wrong and none produce the same result. You may want to have a look at [this post](https://pouchdb.com/2015/05/18/we-have-a-problem-with-promises.html). — Quentin Roy, Nov 23 '16 at 02:16
The last one is very wrong. It calls `xmlToObject(xml)` immediately in order to pass its *return value* to `.then`. It will probably throw an error because `xml` is not defined. — Felix Kling, Nov 23 '16 at 02:22
i should have read the code a little closer. was intended as general advice. you're right, it doesn't apply here. — I wrestled a bear once., Nov 23 '16 at 02:25
@Felix Kling, I added `xmlToObject(xml)` by mistake, it should be `xmlToObject` without xml parameter, thanks — YouneL, Nov 23 '16 at 02:48
At Revision 3, the 4th version went from worst to best. Such a change is confusing as it invalidates some of the answers and comments already posted. — Roamer-1888, Nov 23 '16 at 03:22
I think there's a problem with the function name "post." Inside it, it calls itself ("make an http post [...]"). I assume the newly defined function should be called something else. — Harry Pehkonen, Nov 23 '16 at 05:43

score 4 · Answer 1 · answered Nov 23 '16 at 02:06

4

You can construct your promise chain w/ those above functions in the following way:

post(url, formData)
  .then(xmlToObject)
  .then(doSomeWork);

function doSomeWork(obj) {
  //do some work
}

answered Nov 23 '16 at 02:06

hackerrdave

6,486
1
25
29

It seems `post` does not return a promise. – Quentin Roy Nov 23 '16 at 02:19

score 4 · Accepted Answer · edited May 23 '17 at 12:18

promise.then(function(obj) {
    return obj
})

is pointless and should be replaced by just promise (omitting the then call).

….then(function(xml) {    
    xmlToObject(xml)
// ^ note the missing `return`
}).then(function(obj) {
    …
});

is the only one in your collection that does not work. If you don't return anything, nothing can be awaited (and no result be obtained), and the next callback in the chain will be invoked immediately with undefined.

post(url, formData).then(function(xml) {
    xmlToObject(xml).then(function(obj) {
        // do some work
    })
})

That does work but is cumbersome. You'll get the indentation pyramid of doom, and by neglecting to return anything from the then callbacks you make it impossible to chain anything to the outer promise. This is especially important for handling errors at the end of the chain.

The correct way of chaining promises is therefore

post(url, formData).then(function(xml) {
    return xmlToObject(xml);
}).then(function (obj) {
    // do some work
})

which is exactly equivalent to the shorter

post(url, formData).then(xmlToObject).then(function(obj) {
    // do some work
});

If you want to do some work using both xml and obj, nesting (with return!) is an option out of many.

score 0 · Answer 3 · answered Nov 23 '16 at 02:07

0

this one is correct

post(url, formData).then( function (xml) {
    return xmlToObject(xml)
}).then( function (obj) {
    // do some work
});

Reason:

when return a promise in promise resolver,You can just continue add .then handler.Maybe promise is designed to use this feature to avoid callback hell

answered Nov 23 '16 at 02:07

Roscoe

19
2

1

You may also just replace the entire body of the first `then` with `xmlToObject`. – Quentin Roy Nov 23 '16 at 02:21

score 0 · Answer 4 · answered Nov 23 '16 at 03:50

i prefer for clear chaining this variant:

return Promise.resolve()
    .then(function () {
        return post(url, formData);
    })
    .then(function (xml) {
        return xmlToObject(xml);
    })
    .then(function (object) {
        return something;
    });

Can I use a promise in .then arguments?

4 Answers4