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

Fix svelte-stories-loader in Windows #133

Merged
merged 1 commit into from
Aug 31, 2023
Merged

Conversation

j3rem1e
Copy link
Contributor

@j3rem1e j3rem1e commented Aug 30, 2023

Fix #131

@j3rem1e j3rem1e added bug Something isn't working patch Increment the patch version when merged labels Aug 30, 2023
@j3rem1e j3rem1e requested a review from hobbes7878 August 30, 2023 19:05
const parser = fileURLToPath(new URL('./collect-stories.js', import.meta.url))
.replace(/\\/g, "\\\\"); // For Windows paths;
.replace(/\\/g, '/'); // For Windows paths;
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe call path.posix instead

@@ -59,7 +59,7 @@ function transformSvelteStories(code: string) {

// throws dedent expression is not callable.
// @ts-ignore
return dedent`${codeWithoutDefaultExport}
return `${codeWithoutDefaultExport}
Copy link
Contributor

Choose a reason for hiding this comment

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

why remove dedent? if we remove it I think you'll need to remove the comments on the lines above this as well as the import

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is a regression with dedent from Storybook v7. It doesn't handle correctly quoted backslash. I have added an example in the referenced issue.

Note that the vite loader doesn't use dedent either.

Copy link
Contributor

Choose a reason for hiding this comment

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

will

source = dedent`${component.substr(start, end - start)}`;
fail on windows as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably. However here where are not generating invalid javacsript. at worst the source will have invalid breaking line, but we really need dedent here.

In fact, this issue is not related to windows and is caused by this commit: 02ce02a

Copy link
Contributor

@benmccann benmccann Aug 31, 2023

Choose a reason for hiding this comment

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

I wonder if it would help to upgrade this project from dedent 1.2 to the latest version (1.5.1 if I recall correctly)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you're ok with this, let's create a new issue to improve the rendering of the source snippet.

I'd like to close this PR, it's the last issue for upgrading my stack to Svelte 4 & Storybook 7.4 ^^

@j3rem1e
Copy link
Contributor Author

j3rem1e commented Aug 31, 2023

Note: All my instances of Storybook work with this patch, SB7.4, Webpack5 and Svelte 4.2

@j3rem1e j3rem1e merged commit 40335c9 into storybookjs:main Aug 31, 2023
1 check passed
@j3rem1e j3rem1e deleted the bug/131 branch August 31, 2023 14:31
@shilman
Copy link
Member

shilman commented Aug 31, 2023

🚀 PR was released in v4.0.1 🚀

@shilman shilman added the released This issue/pull request has been released. label Aug 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working patch Increment the patch version when merged released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Doesn't work on Windows anymore
3 participants