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

Svelte 5 Runes support #1300

Draft
wants to merge 25 commits into
base: main
Choose a base branch
from
Draft

Svelte 5 Runes support #1300

wants to merge 25 commits into from

Conversation

SeppahBaws
Copy link
Collaborator

@SeppahBaws SeppahBaws commented Apr 29, 2024

With Svelte 5 now being RC it's about time to finish this up, so that we have Svelte 5 support right when it comes out.

The main breaking changes Svelte 5 brings for us are Runes, which are incompatible with the old export let stuff that Houdini was generating for the query loads etc.

Todo list:

Feedback welcome!

If you would like to try this in your project, bump the version in your package.json to the latest @canary tag on npm.
Please feel free to give feedback in here on things that maybe are broken or could be done better.

For an example usage of how to use it in runes mode, check out the examples in the docs: https://deploy-preview-1300--houdini-docs-next.netlify.app/guides/svelte-5

Sidenotes

  • I'm not entirely sure whether wrapping the store in a $derived() rune is the best way to go about it. I think ideally we would be able to define a custom rune $graphql(...) or $fragment(...) but I doubt that will happen.
  • Maybe the graphql() and fragment() functions could return a rune, so that the $derived() is hidden from the user. -> users don't need to edit anything about their queries to upgrade. need to experiment whether it would even work...

To help everyone out, please make sure your PR does the following:

  • Update the first line to point to the ticket that this PR fixes
  • Add a message that clearly describes the fix
  • If applicable, add a test that would fail without this fix
  • Make sure the unit and integration tests pass locally with pnpm run tests and cd integration && pnpm run tests
  • Includes a changeset if your fix affects the user with pnpm changeset

Copy link

changeset-bot bot commented Apr 29, 2024

🦋 Changeset detected

Latest commit: d0a7c5f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 7 packages
Name Type
houdini-plugin-svelte-global-stores Minor
houdini-svelte Major
houdini Minor
houdini-adapter-auto Patch
houdini-adapter-cloudflare Patch
houdini-adapter-node Patch
houdini-react Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

netlify bot commented Apr 29, 2024

Deploy Preview for houdini-docs-next canceled.

Name Link
🔨 Latest commit d0a7c5f
🔍 Latest deploy log https://app.netlify.com/sites/houdini-docs-next/deploys/668aba82b7f2da00085445a2

Copy link

netlify bot commented Apr 29, 2024

Deploy Preview for houdinigraphql canceled.

Name Link
🔨 Latest commit d0a7c5f
🔍 Latest deploy log https://app.netlify.com/sites/houdinigraphql/deploys/668aba818644f200085960d2

@aumetra
Copy link

aumetra commented May 1, 2024

I'm working on a full rewrite of a Vue app in Svelte 5 + SvelteKit, so I'd be happy to report any issues that come up.

@SeppahBaws
Copy link
Collaborator Author

There is one broken e2e test atm: the /lists/all route somehow only sends a query refetch every 2 times we press arrow up. Will require some more investigating...

@paulocr
Copy link

paulocr commented May 15, 2024

I am seeing lots of these as I make changes to my code, I am thinking it could be my dependencies:

TypeError: Cannot read properties of null (reading 'HoudiniClient')
    at /Users/paulo/development/vendure/sk5_vendure_frontend/src/client.ts:4:20
    at async instantiateModule (file:///Users/paulo/development/vendure/sk5_vendure_frontend/node_modules/.pnpm/[email protected]/node_modules/vite/dist/node/chunks/dep-vyqc0_aB.js:50057:9)

These are my dependencies:

        "@sveltejs/adapter-auto": "^3.0.0",
        "@sveltejs/kit": "^2.0.0",
        "@sveltejs/vite-plugin-svelte": "^3.0.0",
        "@types/eslint": "^8.56.0",
        "@typescript-eslint/eslint-plugin": "^7.0.0",
        "@typescript-eslint/parser": "^7.0.0",
        "eslint": "^8.56.0",
        "eslint-config-prettier": "^9.1.0",
        "eslint-plugin-svelte": "^2.36.0-next.4",
        "houdini": "0.0.0-20240515214704",
        "houdini-svelte": "0.0.0-20240515214704",
        "prettier": "^3.1.1",
        "prettier-plugin-svelte": "^3.1.2",
        "svelte": "^5.0.0-next.1",
        "svelte-check": "^3.6.0",
        "tslib": "^2.4.1",
        "typescript": "^5.0.0",
        "vite": "^5.0.3"

And here is my client.ts:

export default new HoudiniClient({
    url: 'http://localhost:3030/shop-api',
       throwOnError: {
        // can be any combination of
        // query, mutation, subscription, and all
        operations: ['all'],
        // the function to call
        error: (errors, ctx) =>
            error(
                500,
                `(${ctx.artifact.name}): ` + errors.map((err) => err.message).join('. ') + '.'
            ),
    }
})

For now my solution is to restart my dev server and things go along but I am curious if there is something you can spot that is wrong or if anyone is facing the same issue.

@SeppahBaws
Copy link
Collaborator Author

@paulocr hmmm strange, at first glance it looks like it's an error coming from a vite dependency. Can you try updating your vite version to the latest?
I haven't had enough time yet to mess around with svelte 5 enough in our project at work since other higher prio things keep popping up, so if you have a reproduction that would help a lot with tracking this down 🙂

@SeppahBaws SeppahBaws force-pushed the svelte-5 branch 5 times, most recently from 2cc70d5 to f432d9c Compare May 19, 2024 09:53
@SeppahBaws SeppahBaws changed the title [WIP] Svelte 5 Runes support Svelte 5 Runes support Jun 6, 2024
@SeppahBaws
Copy link
Collaborator Author

So I think this PR is not in WIP anymore and is in general stable.
However I'm going to leave the PR as draft until Svelte 5 releases, and put it as ready for review once we can add a dependency on the final 5.0 version.

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.

None yet

4 participants