search

storybookjs / addon-svelte-csf

[Incubation] CSF using Svelte components.
MIT License
103 stars 32 forks source link

fix: simplifying destructing syntax to avoid vite optimisation issue #195

Closed axel7083 closed 1 month ago

axel7083 commented 2 months ago

Following https://github.com/storybookjs/addon-svelte-csf/pull/181. We faced an issue (probably related to vite optimisation) as explained by @xeho91 in https://github.com/storybookjs/addon-svelte-csf/pull/181#issuecomment-2211041666

The problem was linked to the destructuring assignment syntax in the src/runtime/Story.svelte file with the props of the componet.

This PR remove the destructuring assignment in favour of a single variable, this fixing all compilation issue.

xeho91 commented 2 months ago

I'm not sure if this the right solution to the problem.

AFAIK, $props rune return value object properties are meant to be destructed.

I do remember about your issue, and yet I haven't been able to reproduce it on other projects than yours. I need to consider the possibility that maybe the cause of this issue lies somewhere else - the error message and the error stack trace could be misleading. I'll try to investigate again with vite-plugin-inspect tonight my time.

EDIT. No luck, I think it's time to raise this issue with Svelte maintainers. I can't even get close to see a way how vite-plugin-svelte:optimize-svelte transforms the code. Not even with plugin 😞

xeho91 commented 1 month ago

@axel7083

I've got a possible lead. I can't verify right now with podman-desktop repository, but if you could find a time...

I believe the error could be connected with svelte-preprocess. Does the error still occur if you temporarily disable it inside svelte.config.js?

If yes, that'll be good news. Because that'll help us narrow the scope of where the issue occurs. And it might be easier to provide reproduction for Svelte maintainers.

axel7083 commented 1 month ago

I believe the error could be connected with svelte-preprocess. Does the error still occur if you temporarily disable it inside svelte.config.js?

@xeho91 deleting the svelte.config.js file fixes the compilation issue, the problem seems to be linked to the svelte-preprocess library then. Thanks for narrowing down the problem

We are using vite for building the storybook, therefore they may conflict as they have their own preprocess (https://github.com/sveltejs/vite-plugin-svelte/blob/main/docs/preprocess.md)

JReinhold commented 1 month ago

Does this mean that we can close this, or is there still an actual issue hidden somewhere in the addon?

xeho91 commented 1 month ago

Does this mean that we can close this, or is there still an actual issue hidden somewhere in the addon?

Yes, AFAIK the issue is related to vite configuration conflict between svelte-preprocess, and @sveltejs/vite-plugin-svelte.

We can't do anything on our side, nor I have idea how to resolve or provide a simple reproduction.

If it occurs again to someone else, and can provide a simple reproduction, then you'll have a better luck reporting it either to:

In the light of how much I learned and know so far, I'm not sure which one is at fault. Both repositories belong to same organisation, so they will be able to transfer issue in case. But first, a simple reproduction.

In the meantime, I close the PR, as I believe we can agree that it wasn't the right solution to solve the issue.

Thanks for trying and contributing!