Skip to content
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

Add world and context proxies for use in arrow functions #2402

Merged
merged 7 commits into from
May 26, 2024

Conversation

davidjgoss
Copy link
Contributor

@davidjgoss davidjgoss commented May 6, 2024

🤔 What's changed?

This PR adds two new public exports:

  • world which you can reference from a step definition or hook to get the World
  • context which you can reference from a BeforeAll or AfterAll hook to get the context containing the world parameters

I've added this to the docs, but assuming this is successful with early adopters I will raise a follow up PR to switch all our code examples over to use it too, and mark it stable (currently it's beta).

⚡️ What's your motivation?

This provides a solution for the long-standing conflict between wanting to access the "World" instance which has always been bound to this, and wanting to use modern JavaScript syntax with arrow functions that don't honour such a binding.

You can now write step definitions with arrow functions and use the World like this:

const { Given, world } = require('@cucumber/cucumber')

Given('a step', () => {
  world.log(world.parameters.foo)
})

Fixes #2004.

🏷️ What kind of change is this?

  • ⚡ New feature (non-breaking change which adds new behaviour)

♻️ Anything particular you want feedback on?

The exported world object is a Proxy which dynamically forwards to the World for the currently-executing test case, which in turn is provided by a Node.js AsyncLocalStorage which works very similarly to React context in that we can have a piece of state scoped to the running of some code, and reachable from within that code without passing directly.

There are a couple of things to bear in mind with this approach:

  • The proxy is very comprehensive, but it's possible if someone is doing something really weird with their World they could have issues. But then, they always have the old pattern to fall back on.
  • If at a future point we target runtimes besides Node.js, we may have to provide alternate implementations - as such, the use of AsyncLocalStorage here is behind a generic facade and does not leak externally.

📋 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.

This text was originally generated from a template, then edited by hand. You can modify the template here.

@davidjgoss davidjgoss requested a review from mattwynne May 6, 2024 15:09
@coveralls
Copy link

coveralls commented May 6, 2024

Coverage Status

coverage: 97.959% (-0.3%) from 98.234%
when pulling 6787215 on feat/arrows-world
into 2990fe4 on main.

@davidjgoss
Copy link
Contributor Author

Just discussed this with @mpkorstanje who pointed out that an even nicer pattern would be like:

const { Given, world } = require('@cucumber/cucumber')

Given('a step', () => {
  world.log(world.parameters.foo)
})

This could be achievable by using proxies - I'm going to experiment a bit on this branch and see if it looks viable in reality. TypeScript typing would be one potential challenge. But I like it enough to put some time into it!

@davidjgoss davidjgoss changed the title Add getWorld and getContext for use in arrow functions Add world and context proxies for use in arrow functions May 11, 2024
@mpkorstanje
Copy link
Contributor

One last thought. To make API evolution easier, maybe don't put methods like log and attach on world. The world belongs to the user so to say.

And if world is backed by a thread local and passed to another thread (some async call back) what happens?

@davidjgoss
Copy link
Contributor Author

davidjgoss commented May 11, 2024

To make API evolution easier, maybe don't put methods like log and attach on world. The world belongs to the user so to say.

Agreed, we should be able to make log and attach standalone functions hanging off the same mechanism, and ultimately uncouple them from the world contract after a while (it'd be a breaking change today but we'll get there).

And if world is backed by a thread local and passed to another thread (some async call back) what happens?

The scope in which the proxy works wraps the whole of the hook/step execution and explicitly supports async operations happening within it. Once the hook/step execution has finished that world instance is discarded - everything the Cucumber owns that it pushes in is cloned first. You could I suppose siphon off a reference to it outside of your support code and prevent it being garbage collected. But that's kind of the case anyway with this.

@davidjgoss davidjgoss merged commit 1901ec8 into main May 26, 2024
10 checks passed
@davidjgoss davidjgoss deleted the feat/arrows-world branch May 26, 2024 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Arrow function support with world option
3 participants