Replace contructor parameter object with regular arguments

[mattwynne]

🤔 What's changed?

• Replaced the parameter object for Runtime's constructor with a regular list of method arguments
• Used constructor assignment
• Removed the INewRuntimeOptions

⚡️ What's your motivation?

Working on #1770 with @BlueMona was the first time I've really delved into the Cucumber-JS codebase, and the idioms around using constructor parameter objects felt like a hangover from when the codebase was written in JavaScript.

Personally, I think that unless the parameter object has some kind of meaning in the domain, it's better not to have a type for it, and to just pass a list of parameters instead.

I like the fact that we can reduce the boilerplate noise by using TypeScript's constructor assignment feature.

So this is a proposal, for discussion. I think it might be helpful to lighten the codebase a little and reduce the number of types we have to define if we follow this convention more generally. I'd be happy to potter away sending more PRs to do this in other places.

I guess this would end up being a change to the public API in some cases, and therefore might need a major release, so could be a bit disruptive?

🏷️ What kind of change is this?

• 🏦 Refactoring/debt (improvement to code design or tooling without changing behaviour)

📋 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 decreased (-0.0009%) to 98.162% when pulling cc5bb65 on replace-constructor-parameter-objects into 16901cd on main.

[davidjgoss]

Ooh, controversial!

Personally, I think that unless the parameter object has some kind of meaning in the domain, it's better not to have a type for it, and to just pass a list of parameters instead...I like the fact that we can reduce the boilerplate noise by using TypeScript's constructor assignment feature.

I think it depends. Runtime isn't the best example because it's on the public API and people do depend on it (mostly other framework authors, but also some users with niche needs) and I couldn't really get behind a breaking change for purely stylistic reasons.

Options objects in my opinion can be a good choice when:

• There are lots of parameters and many are optional - thus avoiding lots of null or undefined interspersed between the parameters you're actually using
• People read the consuming code and want to understand what's what - the fact that it's an object with intentionally-named keys vs some things in a certain order can make a difference e.g. doThing(stuff, 4, false, true) needs some digging to understand

That said, I think a lot of the internals could benefit from your approach, especially as you say with the TypeScript shorthand for class members from the ctor, and we can refactor with confidence when args are added/removed. The parallel Coordinator I noticed is pretty unweildy in this respect when working on #1931, so maybe that'd be a good one to hit with this.

[mattwynne]

Runtime isn't the best example because it's on the public API

Yes, makes sense. Is the public API surface considered to be anything exported by src/index.ts?

[davidjgoss]

@mattwynne yep exactly