2

I am trying to run the node js Lighthouse function serially (one at a time) with an array of URLs. My problem is that whenever I loop through the array, Lighthouse runs all the URLs at once, which I imagine is problematic if you have a very large array of URLs.

The code:

for(let url of urls) {        
  function launchChromeAndRunLighthouse(url, opts, config = null) {
    return chromeLauncher.launch({chromeFlags: opts.chromeFlags}).then(chrome => {
      opts.port = chrome.port;
      return lighthouse(url, opts, config).then(results => {
        return chrome.kill().then(() => results.lhr)
      });
    });
  }
}

launchChromeAndRunLighthouse('https://example.com', opts).then(results => {
  // Use results!
});

Please help! And thank you for your time!

Jordan Running
  • 102,619
  • 17
  • 182
  • 182
user2305363
  • 157
  • 2
  • 15
  • You shouldn't put a function declaration inside a `for` loop. But despite that, your code doesn't actually use the loop, it just fetches `'https://example.com'` repeatedly... – Patrick Roberts Jan 10 '19 at 15:42
  • Wow messy, take the function outside and consider using a forEach or other array method. Have a look at async/await aswell. You should always favor higher order functions intsead of loops like for ...of. – squeekyDave Jan 10 '19 at 15:52
  • @squeekyDave `forEach()` is not going to fix asynchronous iteration. In fact, it's going to make it harder to implement properly. – Patrick Roberts Jan 10 '19 at 15:58
  • @PatrickRoberts I am not saying forEach will fix everything, as I said, consider other array methods. Also dealing with async code is much much easier with **async/await** so I think he should try that. – squeekyDave Jan 10 '19 at 16:03
  • I'm sorry gang... that code was butchered up above. I got it working. I will post the solution shortly. Thank you for your input! – user2305363 Jan 10 '19 at 16:09
  • Possible duplicate of [Using async/await with a forEach loop](https://stackoverflow.com/questions/37576685/using-async-await-with-a-foreach-loop) – ponury-kostek Jan 10 '19 at 16:25

4 Answers4

4

Your answer is correct but it can be improved. Since you have access to async and await, you should fully utilize it to make your code cleaner:

async function launchChromeAndRunLighthouse (url, opts, config = null) {
  const chrome = await chromeLauncher.launch({chromeFlags: opts.chromeFlags});
  opts.port = chrome.port;
  const { lhr } = await lighthouse(url, opts, config);
  await chrome.kill();
  return lhr;
}

async function launchAudit (urls) {
  for (const url of urls) {
     const results = await launchChromeAndRunLighthouse(url, opts);
     // Use results!
  };
}

launchAudit(urls);
Patrick Roberts
  • 49,224
  • 10
  • 102
  • 153
1

I believe I figured it out. What I did is below. Please continue to send feedback if you think this is wrong.

function launchChromeAndRunLighthouse(url, opts, config = null) {
  return chromeLauncher.launch({chromeFlags: opts.chromeFlags}).then(chrome => {
        opts.port = chrome.port;
    return lighthouse(url, opts, config).then(results => {
      return chrome.kill().then(() => results.lhr)
    });
  });
};

async function launchAudit(urls) {
  for (let url of urls) {
     await launchChromeAndRunLighthouse(url, opts).then(results => {
       // Use results!
     });
  };
};

launchAudit(urls);
user2305363
  • 157
  • 2
  • 15
0

A variation on Patric Roberts answer (which should be the accepted answer).

I was wondering if it was necessary to kill chrome every iteration.

const lighthouse = require('lighthouse');
const chromeLauncher = require('chrome-launcher');

function launchChromeAndRunLighthouse(sites, opts, config = null) {
    return chromeLauncher.launch({chromeFlags: opts.chromeFlags}).then(chrome => {
    opts.port = chrome.port;
    const siteResults = [];
    return new Promise((resolve, reject) => {

        // batch async functions.
        // C/O https://stackoverflow.com/questions/43082934/how-to-execute-promises-sequentially-passing-the-parameters-from-an-array
        const runBatch = async (iterable, action) => {
            for (const x of iterable) {
                await action(x)
            }
        }

        // func to run lighthouse
        const doLightHouse = (site) => new Promise((resolve, reject) => {
            lighthouse(site, opts, config).then(results => {
                siteResults.push(results.lhr);
                resolve();
            });

        });

        // go go go
        runBatch(sites, doLightHouse).then(d => {
            chrome.kill().then((result) => {
                resolve(siteResults)
            })
        });
    });

});
}

const opts = {
    chromeFlags: ['--show-paint-rects'],
    onlyCategories: ['performance']
};

const sites = ['https://www.example.com', 'https://www.test.com']

launchChromeAndRunLighthouse(sites, opts).then(results => {
   // Use results!
    console.log(results);
});
Emile
  • 11,451
  • 5
  • 50
  • 63
-2

Just to execute your code as test, we'll use async/await and IIFE
Then, will create function which will put all our request to array of non resolved promises, so we could use it with Promise.all()
You need to rewrite code in something like this:

(async() => {
  const promisesToExecute = [];

  const launchChromeAndRunLighthouse = async (url, opts, config = null) => {
     const chrome = await return chromeLauncher.launch({chromeFlags: opts.chromeFlags});
     opts.port = chrome.port;

     promisesToExecute.push(lighthouse(url, opts, config));
  }

  const results = await Promise.all(promisesToExecute);

  for(const result of results) {
    const resolvedResult = await result.kill();
    // here you can access your  results.lhr
    console.log(resolvedResult.lhr);
  }
})()

Please note, this code wasn't tested, so there might be problems with kill() on result. But, the main goal is to answer your question and explain how to execute promises.
Also, if you don't want to execute all promises at the same time, you could use Promise.waterfall with some npm package, like this

Grynets
  • 2,477
  • 1
  • 17
  • 41