0

I've created a script that loops through a bunch of folders and processes them each in to webpack bundles. This works great, except that I can't figure out why the Promise around the loop isn't resolving.

Some things I've tried:

  • If I put a console.log("hello world") just before resolve(), within the } else { ... }, it outputs the log.
  • If I move resolve() out of the } else { ... }, it resolves, but the rest of my gulp task doesn't continue (separate, but related, issue).

I would appreciate some help in figuring this out. Most relevant chunk of code is below, the rest of it's at the link below.

// process all the script folders
const process_script_folders = () => {
   return new Promise((resolve) => {
       const FOLDER = script_folders.shift();

       // lint all scripts, except for critical
       if (FOLDER !== "critical") {
           const linted = lint_scripts(js_directory, FOLDER + ".js", source_directory + "/" + FOLDER + "/**/*");
           merged_streams.add(linted);
       }

       process_scripts(js_directory, FOLDER + ".js", source_directory + "/" + FOLDER + "/**/*").then((processed) => {
           merged_streams.add(processed);

           if (script_folders.length > 0) {
               process_script_folders();
           } else {
               // @TODO figure out why this isn't resolving
               resolve();
           }
       });
   });
};

return process_script_folders().then(() => {
    // ... do stuff
    console.log("Testing"); // currently never output
});

https://github.com/JacobDB/new-site/blob/dfeeb3260ab1b314e7562ef313c181adf2ef7f9c/gulp-tasks/scripts.js#L86-L89

JacobTheDev
  • 17,318
  • 25
  • 95
  • 158
  • Do you need to `return resolve();`? – leocreatini Sep 13 '18 at 17:18
  • 2
    Your recursive call `process_script_folders()` doesn't resolve the promise returned by the outer call – Bergi Sep 13 '18 at 17:20
  • @bergi I think I get what your saying, but I'm not quite sure how to handle it. Promises are still new to me; can you provide an example on how to achieve that? – JacobTheDev Sep 13 '18 at 17:39
  • You call the process_script_folders() method recursively and resolve the promise inside another promise. The resolve() will not get called because you do not resolve() your first promise so the then method will not get executed for first one thas wahy your resolve method also not get called. – Anupam Sep 13 '18 at 17:40

2 Answers2

2

You are recursively calling your promise, but none of these promises will resolve aside from the last one because you have put your resolve inside of your resolve clause. I think if you put the resolve outside of else and keep the rest the same it should resolve after finishing the recursion.

if (script_folders.length > 0) {
    process_script_folders();
} 
resolve();

Could you try it like that?

EDIT: @Bergi is right. Doing it like the following should have it working properly I think. First a trivial example of what I'm suggesting you do:

let i = 0
const example_processing = () => {
    return new Promise((resolve) => {
        i++
        setTimeout(resolve, 1000);
    }).then(() => {
        console.log(i);
        return i < 10 ? example_processing() : "done"
    });
};
example_processing().then(console.log);

With regards to your code it would look more like this I guess:

const process_script_folders = () => {
    return new Promise((resolve) => {
        const FOLDER = script_folders.shift();

        // lint all scripts, except for critical
        if (FOLDER !== "critical") {
            const linted = lint_scripts(js_directory, FOLDER + ".js", source_directory + "/" + FOLDER + "/**/*");
            merged_streams.add(linted);
        }

        process_scripts(js_directory, FOLDER + ".js", source_directory + "/" + FOLDER + "/**/*").then((processed) => {
            merged_streams.add(processed);
            resolve();
        });
    }).then(() => script_folder.length > 0 ? process_script_folders() : "done");
 };
Bergi
  • 630,263
  • 148
  • 957
  • 1,375
etarhan
  • 4,138
  • 2
  • 15
  • 29
  • No, that way if resolves *before* finishing the innermost recursive calls – Bergi Sep 13 '18 at 17:25
  • @Bergi You are right. Please check my revised answer. – etarhan Sep 13 '18 at 17:52
  • Calling `process_script_folders` from `setTimeout` won't do any good, but the last snippet is perfect! – Bergi Sep 13 '18 at 17:53
  • @Bergi i put that example just to simulate some processing happening. I realize that having the same function names is a bit confusing, I've editing that now. It was just to illustrate. The last piece of code is my actual proposed solution. – etarhan Sep 13 '18 at 17:54
  • 1
    Ah I see. Still the `setTimeout` should be strictly inside the `new Promise` for that :-) I've converted it into a runnable snippet – Bergi Sep 13 '18 at 18:00
  • Ah, pretty sure I understand what's going on here, and my script is working perfectly now. Thanks much! – JacobTheDev Sep 13 '18 at 18:13
  • glad I could help! with some correction from @Bergi ;) – etarhan Sep 13 '18 at 18:14
-1

I think the issue has to do with the way you are making the recursive call.

Try making it

if (script_folders.length > 0) {
    process_script_folders().then(resolve);
} else {
    resolve();
}
Kevin Aud
  • 378
  • 2
  • 12
  • That's the idea, however this is the [`Promise` constructor antipattern](https://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it) :-/ – Bergi Sep 13 '18 at 17:24
  • If he doesn't want to do it this way, then I think the only option is to do it iteratively instead of recursively. How else would you make it resolve after the recursive call finishes? – Kevin Aud Sep 13 '18 at 17:43
  • No, recursive is just fine. But to do it properly you would `resolve(process_script_folders())`, or even better pass only `resolve` as a callback to `process_scripts` and then do everything else in a `then` callback from which you can `return process_script_folders();`. – Bergi Sep 13 '18 at 17:46