cucumber / cucumber-js

Cucumber for JavaScript
https://cucumber.io
MIT License
5.04k stars 1.09k forks source link

Load ES modules in parallel #2262

Closed michael-lloyd-morris closed 1 year ago

michael-lloyd-morris commented 1 year ago

🤔 What's the problem you've observed?

Current ES6 code is using await in a loop - a bad pattern.

for (const path of importPaths) {
    await importer(pathToFileURL(path))
  }

This means the es modules load one at a time. There's a better pattern.

✨ Do you have a proposal for making it better?

  const importPromises:Array<Promise<void>> = [];
  importPaths.map((path) => {
    importPromises.push(importer(pathToFileURL(path)))
  });

  await Promise.all(importPromises);

📚 Any additional context?

This is a simple optimization. Going to create a branch for it now and issue a PR.

davidjgoss commented 1 year ago

There’s an implicit contract around the loading of support files in a predictable order which affects the order of hooks, so this would arguably be a breaking change.

davidjgoss commented 1 year ago

(Not saying we shouldn't do it - it just probably needs to be a major release.)

michael-lloyd-morris commented 1 year ago

A cli flag perhaps?

--processESModulesInParallel

Default is false for the current, and the next major will switch it to default to true.

Anyway, there is now a pull request for this here: https://github.com/cucumber/cucumber-js/pull/2263

Let's continue the conversation there - I'll close this.