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

Corrected failing example in Vue guide #6761

Closed
wants to merge 1 commit into from
Closed

Corrected failing example in Vue guide #6761

wants to merge 1 commit into from

Conversation

ffxsam
Copy link

@ffxsam ffxsam commented May 10, 2019

This example failed out of the box. Registering every time is necessary, apparently

Issue:
Example in Vue guide threw errors on fresh install

What I did

Added component registration in every .add()

This example failed out of the box. Registering every time is necessary, apparently
@vercel
Copy link

vercel bot commented May 10, 2019

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

Latest deployment for this branch: https://monorepo-git-fork-ffxsam-patch-2.storybook.now.sh

@vercel vercel bot requested a deployment to staging May 10, 2019 15:32 Abandoned
@shilman shilman added this to the 5.0.x milestone May 10, 2019
@shilman shilman added the patch:yes Bugfix & documentation PR that need to be picked to main branch label May 10, 2019
@shilman
Copy link
Member

shilman commented May 10, 2019

@backbone87 @elevatebart can you confirm? seems like a serious regression if this is correct...

@ndelangen
Copy link
Member

ndelangen commented May 21, 2019

@shilman The option for a single string was removed at some point in the past I think.

If it does still work, we should add it in the vue-kitchen-sink example, to make sure it stays working.

These are the option currently available for vue users.
https://github.com/storybooks/storybook/blob/next/examples/vue-kitchen-sink/src/stories/custom-rendering.stories.js#L9-L92

AFAIK, this should work with globally register components:

  .add('template', () => ({
    template: `
      <div>
        <h1>A template</h1>
        <p>rendered in vue in storybook</p>
      </div>`,
  }))

@elevatebart
Copy link
Contributor

I believe the component MyButton is registered globally which makes these changes normally unnecessary.

I will check this immediately

@elevatebart
Copy link
Contributor

elevatebart commented May 21, 2019 via email

@ndelangen
Copy link
Member

Did you find if this is necessary @elevatebart?

@elevatebart
Copy link
Contributor

elevatebart commented May 23, 2019 via email

@elevatebart
Copy link
Contributor

elevatebart commented May 23, 2019

I just checked on next branch and all seems fine.
No need to update the docs.
The pure string story is still working fine in 5.1.0-rc.0.

@ffxsam you seem to have missed - so had I for the record - that in the guide, vue components are meant to be registered globally in the config.js file. It is only mentioned in the details sections. Maybe we should clarify this.

Does it make more sense now?

Capture d’écran 2019-05-23 à 07 32 02

@ndelangen ndelangen closed this May 24, 2019
@ndelangen
Copy link
Member

Thanks for all your detective work 🕵 @elevatebart 👍

@ndelangen
Copy link
Member

@ffxsam thank you for helping! Super appreciated

@ffxsam
Copy link
Author

ffxsam commented May 24, 2019

@elevatebart

I just checked on next branch and all seems fine.
No need to update the docs.
The pure string story is still working fine in 5.1.0-rc.0.

@ffxsam you seem to have missed - so had I for the record - that in the guide, vue components are meant to be registered globally in the config.js file. It is only mentioned in the details sections. Maybe we should clarify this.

Ah, I did see this, but this intention was not clear. It states:

You might be using global components or vue plugins such as vuex, in that case you'll need to register them in this config.js file.

I would consider global components to be from plugins or other third party libraries such as Vuetify and Buefy. I don't consider my own internal components to be global at all (as I import them when necessary).

@ffxsam ffxsam deleted the patch-2 branch May 25, 2019 22:20
@shilman shilman added the patch:done Patch/release PRs already cherry-picked to main/release branch label Jun 6, 2019
@backbone87
Copy link
Contributor

we should update the storybook vue docs and explain what i wrote here: #6357 (comment)

storybook adds no magic regarding component registration

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation patch:done Patch/release PRs already cherry-picked to main/release branch patch:yes Bugfix & documentation PR that need to be picked to main branch vue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants