New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature request: UI that does not use the this
keyword
#745
Comments
I agree that it will mean you always accept |
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
Can't you use the fat arrow inside the step definitions in order to avoid this?
Can you explain this a little more / give an example |
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 ;-) |
When we're defining steps, we're defining functions, not classes, so logically I think this is a situation for the FP paradigm rather than the OOP paradigm. |
@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: |
There's definitely no need to monkey The problem (which isn't a big problem) is that inside a step definition, |
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
}) |
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 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. |
How about we change the 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 Next, we keep the old way of working but generate a |
I'm afraid the argument about compatibility of snippets found online is a compelling argument. Even if there will be 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 |
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 })
}) |
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. |
@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:
If you try to add additional parameters to work around it, you get
|
@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: 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. |
@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. 🙂 |
@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. |
Closing this now, because:
|
Given a world such as:
I want to be able to write:
instead of
This will also rid me from monkeying the
.bind(this)
on all async operations, or if you prefer - rid of monkeying withvar 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
The text was updated successfully, but these errors were encountered: