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

Vue: Add repro template for vue-cli #19165

Merged
merged 37 commits into from
Sep 18, 2022
Merged

Conversation

shilman
Copy link
Member

@shilman shilman commented Sep 11, 2022

Issue: N/A

What I did

  • Added repro template for vue-cli (Vue3)
  • Added repro template for vue-cli (Vue2) but commented out due to Vue2 args handling #19204
  • Fixed a bug with TS handling in Vue3
  • Added the ability to skip certain play functions based on which renderer you're in
  • Prefixed chromatic junit reports with the template-key to make failures easier to debug

How to test

  • CI passes except for existing errors on the next branch

@shilman shilman added the maintenance User-facing maintenance tasks label Sep 11, 2022
@shilman
Copy link
Member Author

shilman commented Sep 14, 2022

Current status: The interaction tests are failing in the Vue2 example. Reproduction:

git checkout shilman/vue-cli-repro-templates
./bootstrap.sh
cd code
yarn sandbox --template vue-cli/vue2-default-js
open http://localhost:6006/?path=/story/addons-interactions-basics--type

What happens is that the Vue2 renderer replaces #storybook-root here: https://github.com/storybookjs/storybook/blob/next/code/renderers/vue/src/render.ts#L42

Which messes up testing-library's within(canvasElement).

In other words, if you change the play function here: https://github.com/storybookjs/storybook/blob/next/code/addons/interactions/template/stories/basics.stories.ts#L22

If you rewrite it to use the global document, it will succeed:

document.querySelector('[data-testid="value"]')

@ghengeveld helped me debug this, and we believe that the Vue2 renderer should probably not be rewriting the storybook-root element.

One solution might be to create a dummy vue2-root element immediately beneath the storybook-root, tho perhaps there is a better way. Would love to pair on this for a few mins with a Vue2 expert perhaps we can figure it out together. I have it all set up on my machine & understand everything except some of the specifics of Vue.

@shilman
Copy link
Member Author

shilman commented Sep 16, 2022

Vue2 args handling

Our framework-independent stories use a pattern where we override args to the Pre component, e.g.

  decorators: [
    (storyFn: PartialStoryFn, context: StoryContext) =>
      storyFn({ args: { object: context.argTypes } }),
  ],

This does not work with Vue2 args handling. We should change the way Vue2 rendering works to make it consistent with the other frameworks.

@shilman shilman mentioned this pull request Sep 17, 2022
@shilman shilman added the vue3 label Sep 17, 2022
Copy link
Member

@tmeasday tmeasday left a comment

Choose a reason for hiding this comment

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

LGTM! Like the skipping approach.

scripts/tasks/chromatic.ts Outdated Show resolved Hide resolved
@shilman
Copy link
Member Author

shilman commented Sep 18, 2022

@tmeasday I've split of the Vue2 changes into a separate PR that needs more work. #19207

@shilman shilman changed the title Vue: Add repro templates for vue-cli Vue: Add repro template for vue-cli Sep 18, 2022
@shilman shilman merged commit eace33c into next Sep 18, 2022
@shilman shilman deleted the shilman/vue-cli-repro-templates branch September 18, 2022 09:59
@@ -40,7 +40,7 @@ Object {
"include": Array [
"ROOT",
],
"test": "/\\\\.(mjs|tsx?|jsx?)$/",
Copy link
Member

Choose a reason for hiding this comment

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

I know this is just a snapshot update, but was the removal of tsx intentional?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance User-facing maintenance tasks vue3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants