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

[2.x] Refactor createInertiaApp in Svelte adapter #2036

Merged
merged 5 commits into from
Oct 18, 2024

Conversation

pedroborges
Copy link
Collaborator

@pedroborges pedroborges commented Oct 15, 2024

While investigating issue #2012 with @saml-dev, I identified the need for more predictable updates to the $page store. This PR ensures that the $page store updates are more predictable to prevent reactivity issues, aligns the Svelte adapter with the other adapters, and simplifies the SSR (Server-Side Rendering) configuration.

Please note that this PR does not fully resolve issue #2012. I will submit a separate PR that builds on these changes to address the remaining aspects of the issue.

Data resolution for the <App /> component has been moved from the createInertiaApp function into the component itself. The internal $store has been removed, and the state it managed is now local within the <App /> component. This change ensures that re-renders and $page store updates occur in a more predictable manner.

Additionally, I've simplified the SSR setup by allowing it to be managed by the user, consistent with the other adapters. This reduces the complexity within the adapter and provides more flexibility for custom server-side rendering configurations. I'll update the docs with instructions on how to add the setup callback to the resources/js/ssr.js file.

I have tested these changes on both playgrounds, including in SSR mode, as well as in two other projects, one using Svelte 4 and another using Svelte 5.

@joetannenbaum I welcome any feedback on how I have set up the Svelte-specific test. While the current test is functional, I know that it can be improved and plan to refactor it when I have the opportunity.

Copy link
Contributor

@joetannenbaum joetannenbaum left a comment

Choose a reason for hiding this comment

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

Thanks for chatting through this with me, looks good.

@reinink reinink merged commit a95409f into master Oct 18, 2024
8 checks passed
@reinink reinink deleted the svelte-improve-createInertiaApp branch October 18, 2024 01:27
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