Skip to content

feat: add support for play functions#57

Merged
tobiasdiez merged 8 commits intotobiasdiez:mainfrom
AntarEspadas:play-function
May 1, 2023
Merged

feat: add support for play functions#57
tobiasdiez merged 8 commits intotobiasdiez:mainfrom
AntarEspadas:play-function

Conversation

@AntarEspadas
Copy link
Copy Markdown
Contributor

Needed #12 for a project, so I figured I might be able to implement it.

Haven't done any extensive testing, but seems to work so far. Main caveat is that the play function must be defined in a non-setup script function.

<template>
  <Stories title="Example/Play function">
    <Story title="Hello world" :play="playFunction">
      <label for="hello">Hello</label>
      <input id="hello" type="text" />
    </Story>
  </Stories>
</template>

<script setup lang="ts">
import { userEvent, within } from "@storybook/testing-library"
</script>

<!-- Notice the play function is in a non-setup script tag -->
<script lang="ts">
async function playFunction({ canvasElement }: { canvasElement: HTMLElement }) {
  const canvas = within(canvasElement)
  const input = canvas.getByLabelText("Hello")
  await userEvent.type(input, ", World!", { delay: 100 })
}
</script>

Copy link
Copy Markdown
Owner

@tobiasdiez tobiasdiez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot, this looks very nice.

Do you have an idea why its necessary to define the play function in a non-setup tag? Is this an issue with the resolveScript method in the parser?

Could you please:

That would be awesome!

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 30, 2023

Codecov Report

Merging #57 (3ce7184) into main (4993db7) will decrease coverage by 0.05%.
The diff coverage is 92.85%.

@@            Coverage Diff             @@
##             main      #57      +/-   ##
==========================================
- Coverage   93.41%   93.37%   -0.05%     
==========================================
  Files           3        3              
  Lines         319      332      +13     
  Branches       51       56       +5     
==========================================
+ Hits          298      310      +12     
- Misses         21       22       +1     
Impacted Files Coverage Δ
src/core/parser.ts 97.12% <91.66%> (-0.52%) ⬇️
src/core/transform.ts 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@AntarEspadas
Copy link
Copy Markdown
Contributor Author

Alright, I'll add the example and the type declaration in a second.

When defining the function in a setup script, this is the code that is generated:

import { defineComponent as _defineComponent } from "vue";

const _sfc_main = /*#__PURE__*/ _defineComponent({
  setup(__props, { expose }) {
    expose();

    function playFunction({ canvasElement }: any) {
      console.log("playFunction");
    }

    const __returned__ = { playFunction };
    Object.defineProperty(__returned__, "__isScriptSetup", {
      enumerable: false,
      value: true,
    });
    return __returned__;
  },
});
export default {
  //decorators: [ ... ],
  parameters: {},
};

function renderPrimary(_ctx, _cache, $props, $setup, $data, $options) {
  return "hello";
}
export const Primary = () =>
  Object.assign({ render: renderPrimary }, _sfc_main);
Primary.storyName = "Primary";
Primary.play = playFunction;
Primary.parameters = {
  docs: { source: { code: `hello` } },
};

As you can see, the play function gets put inside the _defineComponent function, making it inaccessible to the rest of the code. I don't believe it's a bug, but it does mean play functions can't work when defined in a setup scripts.

It might be possible to somehow get the function out of defineComponent, but that seems like more trouble than it is worth.

@tobiasdiez
Copy link
Copy Markdown
Owner

Thanks for the explanation. That makes sense. We might think about properly handling script setup in the future (but I'm not sure how much unpacking should be done). For now, I agree that defining play functions in non-setup scripts is okay.

@AntarEspadas
Copy link
Copy Markdown
Contributor Author

I think that's it

Copy link
Copy Markdown
Owner

@tobiasdiez tobiasdiez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a very nice addition, thanks a lot!

@tobiasdiez tobiasdiez changed the title Add support for play functions feat: add support for play functions May 1, 2023
@tobiasdiez tobiasdiez merged commit e309234 into tobiasdiez:main May 1, 2023
This was referenced May 1, 2023
@AntarEspadas
Copy link
Copy Markdown
Contributor Author

Happy to help! This addon is really neat. I'd love to work on more issues when I get the time

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.

2 participants