Promise: avoid forking/splitting and chain first this then do this

Question

Here's the code as it is now, promise chain is the same it was before. I've started to attempt to use Promise.all() to move away from the antiPattern

(function(){
'use strict';

function DOMObj(){
    var that = this;

    that.items = [];

    that.getitems = function(url) {
        return new Promise (resolve => {
                $.getJSON(url, function (response) {
                    for (var i = 0; i < response.sales.length; i++) {

                        that.items.push(new ProductObj(response.sales[i], i));

                    }
                    resolve();
                });
            }
        )
    };

    that.updateProductHTML = function(){
        return new Promise(resolve => {
            for( var i = 0; i < that.items.length; i++){

                that.items[i].updateHTML();

            }
            resolve();
        })
    };

    that.updateDOM = function() {
        return new Promise(resolve => {
            var thisHTML = '';

            for( var i = 0; i < that.items.length; i++) {

                if (i % 3 === 0 ) {
                    thisHTML += "<div class='row'>";
                    // console.log("START");
                }

                thisHTML += that.items[i].htmlView;

                if ((i % 3 === 2) || i === (that.items.length - 1) ) {
                    thisHTML += "</div>";
                     console.log("FINISH");
                }
            }
            $("#content").append(thisHTML);
            resolve();
        })
    }
}

function ProductObj(product, i) {
    var that = this;

    that.photo = product.photo;
    that.title = product.title;
    that.subtitle = product.subTitle;
    that.url = product.url;

    that.htmlView = "";
    that.index = i;


    that.updateHTML = function() {
        $.get('template.html', function(template){
            that.htmlView = template.replace('{image}', that.photo)
                .replace('{title}', that.title)
                .replace('{subtitle}', that.subtitle)
                .replace('{url}', that.url);

             console.log(that.index + ' product has worked through html')
        })
    };
}
var myView = new DOMObj();

myView.getitems('data.json')
    .then(
        myView.updateProductHTML
    )
    .then(
        myView.updateDOM()
    )
    .then(() =>{
        $('.deleteButton').click(() => {
            $(this).parent().remove();
        });

//Promise.all([ myView.getitems('data.json'), myView.updateProductHTML, myView.updateDOM, () => {$('.deleteButton').click(() => {
    //$(this).parent().remove();
//})}])

})();

so far the code runs in this order getItems => updateProductHTML then updateDOM runs with it, the last code I'm attempting to add is a click event on a button which needs to run last apparently

Please show us your actual code. Don't say "*my promises are set up as such*" and then show us horribly broken (pseudo?) code. — Bergi, Nov 03 '16 at 00:55
That `this.updateHTML` looks asynchronous, yes, but it actually needs to **`return` the `$.get` promise** to be useful! — Bergi, Nov 03 '16 at 00:56
Well, but it's the code you have problems with, so please don't skip it. — Bergi, Nov 03 '16 at 00:58
Oh, sorry for the misunderstanding then. But that cannot work, you never *call* `resolve()`. `updateProductHTML` and `updateDOM` would never run, though you claimed they do? Regardless, please add the code of `getData`, as that's the asynchronous one that you need to fix. Just calling it from inside the promise constructor doesn't help anything, you need to actually fulfill the promise asynchronously. — Bergi, Nov 03 '16 at 01:04
In the code you posted, you have `resolve;` not `resolve()`. — Bergi, Nov 03 '16 at 01:24
ah, you're looking at the old code. The updates were my attempt at keeping an archive of some sort if this is looked up later. I'm just gonna paste the code as it is now — Rob Cha, Nov 03 '16 at 01:28

score 1 · Accepted Answer · answered Nov 02 '16 at 19:12

1

Change your functions to return a promise that will be resolved when the callback function has finished:

this.updateHTML = function() {
    return new Promise(resolve => {
        $.get('product-template.html', function(template) {
            this.htmlView = template.replace('{image}', this.photo)
                .replace('{string1}', this.string1)
                .replace('{string2}', this.string2)
                .replace('{url}', this.URL)
                .replace('{class}', this.class);
            resolve();
        });
    });
};

Same for getData().

Then you should be able to easily chain the steps:

getData().then(updateHTML).then(updateDOM);

answered Nov 02 '16 at 19:12

TimoStaudinger

41,396
16
88
94

Ok with that setup i get getData => updateProductHTML and updateDOM at the same time. I can't drop the promise into the updateHTML as its inside of another for loop so the thenable can't reach it. I put it on my for loop but it's not running right – Rob Cha Nov 02 '16 at 23:13
Avoid the [`Promise` constructor antipattern](http://stackoverflow.com/q/23803743/1048572)! – Bergi Nov 03 '16 at 00:56
@Bergi Interesting read. This applies mainly to `getData` though, as a potential `fetch` already returns a promise, not to `updateHTML`, correct? – TimoStaudinger Nov 03 '16 at 18:32
Your `updateHTML` is using a promise - `$.get(…)`. It does take optional callbacks, but always returns a (jQuery) promise. You can simply `omit` it and even put that callback in a `.then(…)` call. If you don't want to use jQuery promises, [just wrap it in a `Promise.resolve(…)`. – Bergi Nov 03 '16 at 20:06

Promise: avoid forking/splitting and chain first this then do this

1 Answers1