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: simplifying destructing syntax to avoid vite optimisation issue #195

Conversation

axel7083
Copy link

@axel7083 axel7083 commented Jul 29, 2024

Following #181. We faced an issue (probably related to vite optimisation) as explained by @xeho91 in #181 (comment)

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
Copy link
Collaborator

xeho91 commented Jul 30, 2024

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
Copy link
Collaborator

xeho91 commented Aug 9, 2024

@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
Copy link
Author

axel7083 commented Aug 16, 2024

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
Copy link
Collaborator

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

@xeho91
Copy link
Collaborator

xeho91 commented Aug 19, 2024

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!

@xeho91 xeho91 closed this Aug 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants