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.

Sign up for GitHub

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

Jump to bottom

fix(parser): Resolve autodocs tag issue and extracting rawCode #201

Merged
merged 9 commits into from
Aug 22, 2024
Merged

fix(parser): Resolve autodocs tag issue and extracting rawCode #201

merged 9 commits into from
Aug 22, 2024

Conversation

xeho91
Copy link
Collaborator

@xeho91 xeho91 commented Aug 12, 2024 •
edited by shilman
Loading

Resolves #199

Updated reproduction for manual testing in development mode: https://stackblitz.com/edit/sveltejs-kit-template-default-132muy

Fixed:

  1. Analyzing Attribute AST node value when it can be a single ExpressionTag
  2. Walking on compiled Stories component function to extract Story definitions
📦 Published PR as canary version: 4.2.0--canary.201.928fe20.0

✨ Test out this PR locally via:

npm install @storybook/addon-svelte-csf@4.2.0--canary.201.928fe20.0
# or 
yarn add @storybook/addon-svelte-csf@4.2.0--canary.201.928fe20.0

Version

Published prerelease version: v5.0.0-next.2

Changelog

💥 Breaking Change

🐛 Bug Fix

  • fix(parser): Resolve autodocs tag issue and extracting rawCode #201 (@xeho91)
  • chore: use dist folder to load the files #185 (@benoitf)

Authors: 4

@xeho91 xeho91 added bug Something isn't working minor Increment the minor version when merged prerelease This change is available in a prerelease. labels Aug 12, 2024
@xeho91 xeho91 self-assigned this Aug 12, 2024
@xeho91 xeho91 changed the base branch from main to next August 12, 2024 10:06
@xeho91 xeho91 marked this pull request as ready for review August 12, 2024 10:11
@xeho91 xeho91 requested a review from JReinhold August 12, 2024 10:11
@xeho91
Copy link
Collaborator Author

xeho91 commented Aug 12, 2024 •
edited
Loading

@JReinhold

Workflow Check / Test (with svelte-check) failure is expected. Is related to sveltejs/svelte#12292

@xeho91 xeho91 marked this pull request as draft August 12, 2024 10:24
@xeho91
Copy link
Collaborator Author

xeho91 commented Aug 12, 2024 •
edited
Loading

EDIT Resolved

Converting back to draft, it needs further inspection. I was checking only on Chromatic for now.

But then I checked in development mode on my device, and the issue still persists.

@xeho91 xeho91 marked this pull request as ready for review August 12, 2024 11:25
@xeho91 xeho91 changed the title fix(parser): Analysing Attribute AST node value when it can be a single ExpressionTag fix(parser): Resolve autodocs tag issue and extracting rawCode Aug 12, 2024
JReinhold
JReinhold approved these changes Aug 22, 2024
View reviewed changes
Copy link
Collaborator

@JReinhold JReinhold left a comment

Choose a reason for hiding this comment

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

a tiny question about safety, otherwise it looks good!

src/utils/error.ts Outdated Show resolved Hide resolved
src/utils/error.ts Show resolved Hide resolved
src/utils/error/parser/analyse/story.ts Show resolved Hide resolved
xeho91
xeho91 commented Aug 22, 2024
View reviewed changes
src/parser/analyse/story/attributes.test.ts
@@ -37,9 +37,9 @@ describe(getStringValueFromAttribute.name, () => {
.toThrowErrorMatchingInlineSnapshot(`
[SB_SVELTE_CSF_PARSER_ANALYSE_STORY_0001 (AttributeNotStringError): In the stories file: <path not specified>

A '<Story name="<unspecified Story name>" />' has a prop 'name' whose value must be a static literal string.
A '<Story name="undefined" />' has a prop 'name' whose value must be a static literal string.
Copy link
Collaborator Author

@xeho91 xeho91 Aug 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@JReinhold If you're cool with this change, then please feel free to merge it.

And also, I'd like you to ensure if the tags are correct for the next release. I'm a newbie when it comes to using auto. I'm not 100% sure whether it should be major or patch with the prerelease of the next branch.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm actually also similarly confused about it. We'll see what happens. 🚀

Copy link
Collaborator

Choose a reason for hiding this comment

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

Phew https://github.com/storybookjs/addon-svelte-csf/releases/tag/v5.0.0-next.2 😅

I guess it's just the canary versioning that's a little broken with auto.

@xeho91 xeho91 added patch Increment the patch version when merged and removed minor Increment the minor version when merged labels Aug 22, 2024
@JReinhold JReinhold merged commit 01768c8 into next Aug 22, 2024
6 of 7 checks passed
@JReinhold JReinhold deleted the fix-attribute-parse branch August 22, 2024 18:42
@shilman
Copy link
Member

shilman commented Aug 22, 2024

🚀 PR was released in v5.0.0-next.2 🚀

@shilman shilman mentioned this pull request Aug 27, 2024
Merged
@xeho91 xeho91 mentioned this pull request Sep 5, 2024
Closed
@xeho91 xeho91 mentioned this pull request Sep 5, 2024
Merged
This was referenced Oct 16, 2024
Merged
Merged
Merged
This was referenced Oct 25, 2024
Merged
Merged
Merged
@shilman shilman mentioned this pull request Nov 5, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JReinhold JReinhold JReinhold approved these changes

Assignees

@xeho91 xeho91

Labels
bug Something isn't working patch Increment the patch version when merged prerelease This change is available in a prerelease.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug]: autodocs using svelte-csf v5 does not render individual story within docs
3 participants
@xeho91 @shilman @JReinhold