-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
feat: svelte 5 adapter #5403
feat: svelte 5 adapter #5403
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good
The |
Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>
…able into feat-svelte-5-adapter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉
Yeah, we will not just remove Svelte 3/4 support. Especially while Svelte 5 is a beta.
I was originally against this because of how messy it seemed. But it might be the best way forward. I can see table having "svelte-runes", or "vue-vapor", etc. adapters. An issue that we would run into, however, is how to install multiple versions of Svelte in the TanStack Table replo. Probably possible with pnpm, just need to look into it. @lachlancollins was telling me if should absolutely be possible to support Svelte 3-5 in one adapter though. I just don't know how to get that to work with flexRender, unless we just deprecate that in the current adapter. |
I'd just keep it simple, create a new package with preview suffix targeting 5.0, can move those bits to the old package once released and bump major. If users need to stay on the old svelte 3-4, don't upgrade to a new major. Also, could just have multiple branches each with different versions / releases with same package name? |
Imo, it's going to be pretty negligible. At least for state management. The state management layer between Svelte (3/4 or this 5 adapter) is pretty thin. There may even be some ways that observables (from Sv 3/5) integrate w/ @tanstack/table a little more intuitively than signals.
I know this is more of a question for the maintainers, but I am afraid I won't have the margins to help out with an adapter that's available across the major versions (3-5) anytime soon. Assuming someone else doesn't pick that up, 2 adapters would probably mean Svelte 5 compatibility comes sooner for folks who are testing it out. I'm also not opposed to publishing it myself as an "unofficial community adapter" of sorts. Also also - the new, easier programmatic rendering made available in Svelte 5 makes the adapter less of an "adapter" and more of an "implementation" if that makes sense. What I mean is - It's simple enough to just be a documented implementation tbh.
I totally understand your frustration, and it's why I started working on this PR. I'm excited to see this available to other svelters. But if this is a blocker for your project, then it's possible to use this adapter before this PR is merged. This new adapter is actually really really tiny, straightforward, and, at least for the time being, you could easily drop its code into your existing project. I basically built the adapter in a SvelteKit project, then copy-pasted it into this codebase. I don't see any reason you wouldn't be able to do the same, assuming there aren't any breaking releases from Svelte 5 since the version I developed on. |
TanStack Table should NOT be a blocker for your team at all. You can just install Table core and make your own Svelte adapter. Copy it from this PR even. This is always an option. |
I don't think two adapters will help much. Supporting 3-5 should be possible if it's ok to have a breaking change to use the FlexRender component introduced in this PR. If so, then the store API can be reintroduced. Once Svelte 5 is stable another major could be released to switch over from store API to runes API (basically this PR). That way the newest version of the adapter works for Svelte 5 and takes advantage of its new features, while the previous version will work for everyone using Svelte 3/4, too. |
@dummdidumm i noticed that release 85 of svelte 5 has a breaking update to |
No |
Looks like there are conflicts in this pr. |
header.column.columnDef.header, | ||
header.getContext() | ||
)} | ||
<FlexRender |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I do this in my project with this adapter source imported I get
Type 'ColumnDefTemplate<HeaderContext<TData, unknown>> | undefined' is not assignable to type 'undefined'.
Type 'string' is not assignable to type 'undefined'.ts(2322)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you could provide a minimal reproduction, that would be very helpful.
4d2ccbc
to
c7c4419
Compare
…able into feat-svelte-5-adapter
I can't provide any feedback as pr was closed but was also seeing: not sure if any is banned in this project but I'm getting errors source importing and trying out.
Feels like the typing here should be inferred from the context, I'm trying to simplify. I'm in the process of trying it out by converting: https://github.com/exceptionless/Exceptionless/blob/main/src/Exceptionless.Web/ClientApp/src/lib/components/data-table/data-table-body.svelte#L44 |
@niemyjski Sorry that was on accident. It's open again. |
See this diff. This narrows the type of content based on context. Not saying it's perfect, but without a minimal repro of the issue it's difficult for me to understand and address your specific case. |
Hey @KevinVandy - may we re-open this discussion in anticipation of v5's official release? We discussed some options on releasing the adapter earlier:
|
My vote is just to replace it, use the current version if you need 4.x |
We can go ahead and merge this into the alpha branch soon, but the documentation for svelte state and adapter needs to be updated with this. |
Overhauls the Svelte adapter so that it's compatible with Svelte 5.
Some overview of the work that was done:
svelte/internal
are removed.createSvelteTable
now returns a signal in lieu of a store. According to @dummdidumm, Stores may eventually be deprecated.flexRender
API is now replaced with a more idiomaticFlexRender
component.FlexRender
employs the use of Svelte 5's new Snippet feature - it made this adapter way simpler.Feedback welcome.
solves #5213