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

Vue3:(feat) add source decorator vue template and setup script + supports of multi slots #20498

Merged
merged 44 commits into from Jan 16, 2023

Conversation

chakAs3
Copy link
Contributor

@chakAs3 chakAs3 commented Jan 4, 2023

Issue: #13917

What I did

I have added a source decorator to renders/vue3 to show a pretty and practical source, multiple components are supported also, I have added source decorators.tests as well, to cover most cases.
image

source supports slot default and named slot, however, I checked the current render is not handling the slot in default Story, you still have to pass the template here, I may be working on this part also if you want @shilman

How to test

  • Is this testable with Jest or Chromatic screenshots? yes with jest and chromatic
  • Does this need a new example in the kitchen sink apps? no I don't thinko
  • Does this need an update to the documentation? no does not

If your answer is yes to any of these, please make sure to include it in your PR.

@chakAs3 chakAs3 changed the title Vue3:(feat) add source decorator to vue3 with jest test Vue3:(feat) add source decorator vue template and setup script + supports of multi slots Jan 4, 2023
@chakAs3
Copy link
Contributor Author

chakAs3 commented Jan 5, 2023

I have add a optional parameter docs.source.withSetupScript if set true, the source generated with setup script

by default if user does not set this params -->

image

if user sets this param
image

source will be like this ->

image

@rmortes
Copy link

rmortes commented Jan 10, 2023

Absolute newbie here: Shouldn't the docs state @click="onClick" instead of :onClick="onClick"?

@chakAs3
Copy link
Contributor Author

chakAs3 commented Jan 12, 2023

@chakAs3 please ping me when this is ready for review and green in CI, I'll happily take a look.

Thanks, @ndelangen I appreciate it. It is green 👍 now. actually, i detected 1 bug vue-docgen-api was driving me mad, even though I upgraded to the latest one. it is not able to parse properly the props from Vue component file with script setup if you pass them as Reference to to defineProps(<Props>())
for now, I put the props directly on CLI templates, until we fix this issue ( I may do a PR, on their repo if I get time )

@ndelangen
Copy link
Member

ndelangen commented Jan 12, 2023

@shilman I'm inclined to approve and merge, WDYT?

@chakAs3
Copy link
Contributor Author

chakAs3 commented Jan 15, 2023

hi @ndelangen @shilman. I removed the 'prettier' package from my deps, actually, I found out that SourceContainer handles the formatting pretty well, I had just to pass the preset to the handler function. and then kind of lazy load prettier from global deps to parse the vue template from the storyFn().

@ndelangen ndelangen merged commit bda4b80 into storybookjs:next Jan 16, 2023
@kasperpeulen
Copy link
Contributor

@chakAs3 Awesome work!

@ndelangen
Copy link
Member

@chakAs3 I was comparing dist output sizes, and noticed a huge difference in the dist of render/vue2 and renderer/vue.
Where renderer/vue was a lot bigger.

This is likely due to this line here:

// eslint-disable-next-line import/no-extraneous-dependencies
import parserHTML from 'prettier/parser-html';

and here:

const ast = parserHTML.parsers.vue.parse(renderFn.toString());

Can we change that code to not depend on prettier?

@ndelangen
Copy link
Member

At the very least if we're not doing the above, we need to add prettier as a devDependency to the package.
The bundler is now bundling prettier is 'by accident' but this isn't clear from the package.json.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants