Make context-in-play-function aware of context variable name

[beaussan]

Issue: #118

What Changed

Previously, the context-in-play-function was forcing the context passthought to be named exactly context. This PR introduces changes to allow for any variable name to be used.

So now, given this, the linter will not fail

export const SecondStory = {
  play: async (ctx) => {
    await FirstStory.play(ctx)
  }
}

Checklist

Check the ones applicable to your change:

• [x] Ran yarn update-all
• [x] Tests are updated
• [x] Documentation is updated

Change Type

Indicate the type of change your pull request is:

• [ ] maintenance
• [ ] documentation
• [x] patch
• [ ] minor
• [ ] major

[yannbf]

Hey @beaussan this is so awesome, thanks a lot for making this contribution!! I only have one small worry, which is the following scenario:

export const SecondStory = Template.bind({})
SecondStory.play = async ({ canvasElement, ...ctx }) => {
    await FirstStory.play({ ...ctx }) // play function would fail, as canvasElement is not passed
}

I don't know if it's a good idea to allow that scenario to work, given that there's a high change that the subsequent play function will fail. I know it will only fail if it actually uses canvasElement, but this could open possibility for errors. WDYT?

[beaussan]

Hey @yannbf , thanks !

It was already allowed previous to my change, but I agree this should error out too.

I could use the same system to extract all of the destructed args from the parent signature to make sure all are passed, WDYT ?

[yannbf]

It was already allowed previous to my change, but I agree this should error out too.

Good point! I tried to reproduce the error in the current version of the plugin, and I believe it will be fine for JS projects but will error in Typescript, as canvasElement is needed as part of the type, so I think that's good enough? Unless you'd like to improve the PR, of course!

[beaussan]

I think it should be good enough, typescript will caught it, and for most js users this will cover most cases.

Because given this :

export const SecondStory = {
  play: async ({ canvasElement, ...ctx }) => {
    const canvas = canvasElement;
    await FirstStory.play({ canvasElement: canvas, ...ctx })
  }
}

It shouldn't error out, but this will be really tricky to check and in my opinion, there could be higher case where a smarter rule fail on valide cases vs the current state of the lint rule

So I would say we can keep it this way and merge it, but I'll you decide @yannbf =)

[yannbf]

Alright! I agree with you. This looks and behaves great! Thank you so so much for your contribution!

[github-actions[bot]]

:rocket: PR was released in v0.6.11 :rocket: