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

feat: add support for play functions #57

Merged
merged 8 commits into from
May 1, 2023
Merged

feat: add support for play functions #57

merged 8 commits into from
May 1, 2023

Conversation

AntarEspadas
Copy link
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
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!

src/core/transform.ts Outdated Show resolved Hide resolved
test/index.test.ts Outdated Show resolved Hide resolved
@codecov
Copy link

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
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
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
Contributor Author

I think that's it

Copy link
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
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