Typescript: setWorldConstructor with anonymous function too restrictive

[dmeehan1968]

👓 What did you see?

Whilst its not included in the documentation, setWorldConstructor can take either a class name (which is a function), or an anonymous function. This later ability is used numerous times in the features to set custom values on the world instance, conveniently ignoring to assign the props to the World instance, as its not relevant in the context of the test case.

For example:

setWorldConstructor(function(props) {
   this.foo = 'bar'
})

When using a custom class extending World, one would pass the props argument to super so that the initial props are initialised on the instance.

When using an anonymous function, this is a plain object, and props is an object containing the initial properties (attach, log and parameters).

Without any type annotation, Typescript prevents assignment to this because it is implicitly any (assuming noImplicitAny is used in tsconfig.

Without a type annotation one gets no protection against typos.

To ensure that Typescript has the correct type for this, the convention in Typescript is to annotate this as the first parameter, and use the IWorld type to infer the correct members. One can use interface merging to add any custom properties.

setWorldConstructor(function(this: World & { foo: string }, props: IWorldOptions & { custom: any }) {
    this.foo = 'bar'
    // props.parameters is correctly typed to indicate a 'custom' member, supplied via the CLI (or cucumber.js)
})

The problem here is that one would need to individually apply the props members to this or they get lost. But we can't use simple assignment as the IWorld interface declares them as readonly. We can assign custom properties aside from those.

Whilst the types allow for assignment to members of this.parameters, this is alterating a by reference object which will affect later scenarios.

The only solution is to use Object.assign to apply the default props and any custom values, eg:

setWorldConstructor(function(this: World & { foo: string }, props: IWorldOptions & { custom: any }) {
    Object.assign(this, props, { foo: 'bar' })
})

Firstly, this just discards the type safety acquired so far.

Secondly, because Object.assign is a shallow copy, it also means that if we wnat to inject something into parameters it will affect subsequent scenarios (although World is recreated, we are working on the original (by reference) copy of the world parameters held by Cucumber from the CLI. We can work around this by using spread to create a new parameters object to override the one passed in props (necessary as assign doesn't merge), thus:

setWorldConstructor(function(this: World & { foo: string }, props: IWorldOptions & { custom: any }) {
    Object.assign(this, props, { parameters: { ...props.parameters, custom: 123 }})
})

✅ What did you expect to see?

I think this is somewhat semantic, as if one had sufficiently complex functionality one probably would use a class instead of an anonymous function, but this seems to highlight a possibly unintended side effect of how the object is created when an anonymous function is used, given the restrictive typing of World (which is probably actually correct elsewhere).

For the most part I wanted to document it in case anyone else came across the same problem, rather than to propose/ask for a fix.

A solution that I can see is that if the instance created by the anonymous function is a plain object it could be merged with an instance derived from World, but that might have its own complicated side effects.

📦 Which tool/library version are you using?

• Cucumber 7.3.2
• Typescript 4.6.2
• ts-node 10.7.0

🔬 How could we reproduce it?

See above examples

📚 Any additional context?

In my use case I was passing in the name of a class from the world parameters, and wanted to convert it to the constructor within the world parameters. The initial mystery was why the props were not being passed into the World instance (because of not using the World constructor), and then how to make changes to them once the types have been added.

The following is equivilent to how I dealt with it

setWorldConstructor(function(this: World, props: IWorldOptions & { model: string }) {

    const models: { [key: string]: any } = {
        Foo,
        Bar,
    }

    // NB: Can't do the following because parameters is an object by reference, which would affect later scenarios
    this.parameters.model = models[props.parameters.model]    // <-- danger!  Unintended side effects

    // NB: And we can't assign a new object as IWorld marks it as readonly
    this.parameters = { ...props.parameters, model: models[props.parameters.model] }    // <-- compile error - attempt to assign to readonly

    // Only way to update this with props and custom is to use Object.assign, but we lose type safety
    Object.assign(this, props, { parameters: { ...props.parameters, model: models[props.parameters.model] }})
})

[davidjgoss]

If you want to use a function then annotating World as this doesn't make sense as it's not going to be an instance of that. Roughly from what you've said:

interface MyWorld extends IWorldOptions {
  foo: string
}

function MyWorldCtor(this: MyWorld, props: IWorldOptions) {
  Object.assign(this, props)
  this.foo = 'asdhasd'
}

setWorldConstructor(MyWorldCtor)

Given('something happens', async function (this: MyWorld) {
  // something
})

Really though I'm not sure what problem you're solving by using a function rather than just having a class that extends the builtin World.

[dmeehan1968]

I did actually realise that by annotating this as World was wrong, because its a plain object on entry. In fact, in can be annotated as:

setWorldConstructor(function(this: IWorldOptions, props: IWorldOptions) {
    // ...
})

because at least that means that attach, log and parameters are no longer readonly and so can be set from props using regular assignment. Alternatively a cast could be used to indicate the intent.

I agree that class is what's documented, even if the features often use the function style because it suits what they are doing in that context. I think the realisation that this is not World on entry (either by construction or interface) is what helped my to clarify what I was doing.

I think this is partly about the different between identity (this is not an instance of World because it wasn't constructed from the World constructor) and interface (this is a plain object that takes on the interface of IWorldOptions, plus anything else I wish to add). It's not clear whether the object must have that interface for the rest of Cucumber to be happy, and that's where I think it was ambiguous whether that object must adhere to World, or any other shape I care for in my tests.

[davidjgoss]

Yeah I think this would benefit from more docs around the setWorldConstructor, I'll run something up for that.

[davidjgoss]

This has now been released in v8.1.0 on npm

https://www.npmjs.com/package/@cucumber/cucumber