Cucumber-js: Feature request: UI that does not use the `this` keyword

[osher]

Given a world such as:

require('cucumber').defineSupportCode( ({setWorldConstructor:world}) => {
    world(class { 
      constructor(p) { ... }
      foo(bar) { return new Promise(.... ) }
    })
})

I want to be able to write:

  When(/^I do the foo with (.*)$/i, (ctx, bar) => ctx.foo(bar) );

instead of

  When(/^I do the foo with (.*)$/i, function(bar) {
      return this.foo(bar)
  });

This will also rid me from monkeying the .bind(this) on all async operations, or if you prefer - rid of monkeying with var that = this.

One more benefit (IMHO)- it will help people get off inheritance tree for worlds and orient them better towards the better powers of JS

[osher]

I agree that it will mean you always accept world as argument, and that's an API change.
I think that like mocha wich has --ui exports and --ui tdd and --ui bdd - the cucue can as well.
It's basically a project variable that determines if steps are called/applied on the world, or accept it as 1st argument.

[charlierudolph]

I really don't think this would be worth it in the long run. Yes its nice for those 1 line steps where you get to use the => where you otherwise be unable to. In my experience I've had very few steps like this. I'm surprised ES6 didn't include an arrow function that does not retain context as that would be ideal to me.

This will also rid me from monkeying the .bind(this) on all async operations, or if you prefer - rid of monkeying with var that = this.

Can't you use the fat arrow inside the step definitions in order to avoid this?

One more benefit (IMHO)- it will help people get off inheritance tree for worlds and orient them better towards the better powers of JS

Can you explain this a little more / give an example

[osher]

In simple words: I belong to a school of programmers that have shunned the way of phrasing a context as a class. The more people get exposed to the dark side of OOP this school grows bigger - and the movement in the JS community that prevented arrow function from retaining context is an example of the strength of this movement.

Without getting into a religious debate - I'll just say I believe in live and let live, you want to use classes? great. Please don't force me to :-)

My general feeling is that it could be a small change, and I'll be happy to do it if you can provide me the general direction of how you think it would be done best ;-)

[mattbrunetti]

When we're defining steps, we're defining functions, not classes, so logically this has no place in it.

I think this is a situation for the FP paradigm rather than the OOP paradigm.

[mattbrunetti]

@osher I think we can give @charlierudolph the benefit of the doubt that he doesn't want to force you to use classes. That said, he shouldn't be forced to integrate (breaking) API changes wherever he's using this package, unless it offers some tangible advantage.

@charlierudolph I think by "the better powers of JS" @osher means FP, Functional Programming.

IMHO this change would make step definitions a teeny bit simpler and more logical. It would basically just remove this line: const thisWorldNotThisStep = this

[mattbrunetti]

There's definitely no need to monkey .bind(this) to all your async functions, and as long as within the step definition you're using arrow functions, no need to use the const self = this pattern.

The problem (which isn't a big problem) is that inside a step definition, this refers to the world, not the step, which is counter-intuitive.

[charlierudolph]

Okay. As not everyone is a functional programmer though and to prevent the introduction of a breaking change, what do you guys think of making an option which makes the world be passed as the first parameter?

defineSupportCode(({setWorldInjectionStrategy}) => {
  setWorldInjectionStrategy('this') // default
  setWorldInjectionStrategy('argument') // makes world be the first argument to steps and hooks 
})

[armandabric]

I totally agree with the idea to inject the world/context has a parameter of the step. Starting by this project, all the JS ecosystem is moving to fully embrace the new functionalities allowed by ES6 and that's a good things ❤️

If you look on other tools like Express or Koa, they set the current request context as the this AND as first parameter of the middleware (it is the equivalent of cucumber step). This solution allow traditional use of function and the use of ES6 arrow function.
Another advantage of the context in as the first parameter is that

For the solution proposed by @charlierudolph, I do not think this could works on a long run: this will split the community in two. All cucumber examples will not be working in all installations and the docs will be duplicated in two. Not fancy.

I'm not sure about the breaking change worries, the v2 is the perfect time to introduce this kind of change. Waiting will force to make/wait for another major.

[nicojs]

How about we change the Before and After to inject world:

defineSupportCode(function({After, Before, Given}) {
  let world;

  // Asynchronous Callback
  Before(function (w, scenarioResult, callback) {
    world = w;
    callback();
  });
  After(function (world, scenarioResult, callback) {
    callback();
  });

  Given('I access the world', () => {
    assert(world); // Yay!
  });
});

That way you can capture the world in a variable an a before step. It won't mess up each step definition.

Next, we keep the old way of working but generate a deprecated message. Or even don't do that, just leave the option there for people that want to use that approach and know what they are doing.

[osher]

I'm afraid the argument about compatibility of snippets found online is a compelling argument.

Even if there will be --ui flags, or setWorldInjectionStrategy('argument') - it becomes a gotcha that is better communicated as a breaking change of a major version, served with all the online discussions and uproar befitting such changes, and helping to eliminate confusion.

So I vote for doing it in the next version of cuke, and ...makedoing until it releases.

A canary version or a hidden flag would be an awsome promotion I'd use and feedback early on

[jbpros]

I'd love to expose an alternative FP interface.

What about the following? (using async/await to illustrate it's promise-friendly too)

import {initialize, Given, Before} from 'cucumber/fn'

// specify a function that returns the initial context:
initialize(async () => ({ a: 42 }))

Before({ timeout: 10 }, async (ctx) => {
  await doStuff(ctx.a)
})

Given(/^a step passes with {number}$/, async (ctx, number) => {
  const newA = await computeStuff(ctx.a, number)
  // tell cucumber about the new context:
  return Object.assign({}, ctx, { a: newA })
})

[jbpros]

I've hacked together cucumber-fp that offers functional step definitions. I've got a few improvement ideas already. Feedback welcome!

I suggest we close this issue and let people experiment with that small lib. Maybe one day we can bring it into Cucumber.js.

[andyearnshaw]

@jbpros I'd prefer not to have to install yet another dependency just so I can use arrow functions in my tests.

This should actually be really simple for everyone to implement themselves. The following code snippet works pretty well (functionally) for me, with a huge caveat that makes it unusable:

// Don't rely on `this` in step definitions. It's 2021 for crying out loud.
const definitionFunctionWrapper = (fn) =>
    function(...args) {
        return fn(...args.slice(0, -1), this);
    }

That caveat being every single step definition now logs the following error because of the additional parameter:

    Error: function uses multiple asynchronous interfaces: callback and promise
       to use the callback interface: do not return a promise
       to use the promise interface: remove the last argument to the function

If you try to add additional parameters to work around it, you get

function has 3 arguments, should have 1 (if synchronous or returning a promise) or 2 (if accepting a callback)

All cucumber-js needs is an option to disable function arguments checks and this problem will go away. Actually on second thought it looks like the test will time out too, as cucumber assumes that a callback is required based on the number of arguments in the step definition function. This could be worked around by giving precedence to a returned promise, though.

[jbpros]

@andyearnshaw thanks for your input, I hear your concern about a dependency "just for arrow functions in stepdefs". This library is basically the solution I use personally to get stateless step defs, I just packaged it for anyone interested out there.

Consider it as a temporary solution around the ongoing debate for such a pure stepdef API in core (it's been going on for more than 4 years, believe it or not). As I said, this is an experiment that could land in core at some point and I would really appreciate feedback from people actually using it. If it gets enough traction, that'd make a better argument for integrating in cucumber.

Also, please note it is offering a couple of other (small) useful functional tools: tap() and enforced read-only contexts.

The arity check on stepdef functions is definitely a problem that I had to circumvent in this lib (in quite an ugly manner). An option to turn it off both on the CLI and programatically would be very useful for this (and potentially other use cases). I'd love to do that, but time is a scarce resource for me at the moment.

Feel free to get inspiration from cucumber-fp to fix the arity check in the meanwhile.

[andyearnshaw]

@jbpros that's great, and I really do appreciate the effort you have put in there. I was more in disagreement with the sentiment that this issue should be closed. I'll take a look at your library and see if it helps me work around that annoying check. 🙂

[jbpros]

@andyearnshaw ha right, thanks for the clarification. I agree we shouldn't ditch this idea and keeping this issue open is probably a good way to keep things transparent, indeed.