Issue 2262 ES imports in parallel

[michael-lloyd-morris]

🤔 What's changed?

Optimization in the support.ts file to gather the dynamic import Promises into an array and then resolve them with a single Promise.all() call.

⚡️ What's your motivation?

This should yield a noticeable speedup on Cucumber installs with very large ES step definition libraries. Other Cucumber installs will be unaffected.

🏷️ What kind of change is this?

• 🐎 Optimization - A change meant to make the code more performant.

♻️ Anything particular you want feedback on?

If there are other instances in the codebase where an await is inside a loop then this pattern should be applied there as well.

📋 Checklist:

• I agree to respect and uphold the Cucumber Community Code of Conduct
• I've changed the behaviour of the code
  • I have added/updated tests to cover my changes.
• My change requires a change to the documentation.
  • I have updated the documentation accordingly.
• Users should know about my change
  • I have added an entry to the "Unreleased" section of the CHANGELOG, linking to this pull request.

[coveralls]

Coverage: 98.561% (+0.0006%) from 98.561% when pulling 5a9d473 on michael-lloyd-morris:issue-2262-es-parallel into 627030d on cucumber:main.

[michael-lloyd-morris]

There is a behavior change in that the step files resolve simultaneously, but I can't imagine it mattering because Cucumber never gave the end users a way to control the order their step files are resolved in. This might be a concern so I sharded this optimization off to it's own PR for review.

[michael-lloyd-morris]

@davidjgoss raised the important point that there may be code out there relying on this sequential load behavior. I'm not so sure this is the case, but that is the safest approach to this optimization.

That would mean delaying this to the next major release and/or using a cli or config flag to allow opt in.

I'm inclined to follow David's advice and just defer to the next major, especially since require imports are going to remain sequential as that is the nature of the beast. Also, the optimization isn't worth the headache except perhaps for the very largest of test libraries.

[davidjgoss]

To be clear I do think this is the right thing to do. With require being synchronous there was never a way to load in parallel but it's one of the big pluses of ESM so we should lean into it. The potential non-deterministic order of Before etc hooks running could be a surprise to some users so I think we should do it in a major to be cautious. Our advice to users wanting to ensure consistent order of hooks running should be to have them in one file.

[michael-lloyd-morris]

Agreed, so flag for version 10 and I'll come back to this when work on that major starts to catch up anything that fell behind? How do I do that or do you want to do it?

[davidjgoss]

Noting that this would, as it stands, break the parallel runtime because that depends on support code being registered in a consistent order across coordinator and workers. We'll need to try and find another way around that in order to go ahead with this change.