Pass live api through context not a prop

[vonagam]

Right now live api is exposed as a prop. And we pass it to all rendered components, even those who do not ask for it, which causes svelte to complain about it.

A viable alternative is to pass it not as a prop, but through context. Which will remove the warnings and ease access for nested components to live api. It is a breaking change through.

// before
export let live

// after
import {getLive} from "live-svelte"
const live = getLive()

getLive is typed which is also a bonus.

To make it work, two and half lines with setupLive call have to be added to app.js (so no action from new users, but migrating users need to add them):

import {getContext} from "svelte"
import {getHooks, setupLive} from "live_svelte"
setupLive(getContext)

Edit: updated the description to reflect current state.

[woutdp]

First impression is that I like this, haven't looked at the code but the idea is sound.

Nested components might need access to the live api, currently you need to pass it manually through every prop, or use a context yourself. This solves that.

Also, conceptually it's a more sound idea than using props.
Props should be data, interface code should be accessed as a library, not passed around as a prop.

I also prefer the latter solution, using a context still feels like it's data instead of code.

Let me think about it a bit again before committing to this :)

[woutdp]

As a counterargument, export let live is dead simple. Context makes it slightly more complex, it's a small thing but I do like the UX at the moment of simply doing export let live.

Using it as a library (util) is the best solution though. Conceptually best, and easy to understand.

[vonagam]

Let me think about it a bit again before committing to this :)

Of course, no pressure, I just saw Component api page for unrelated reason and when it came to context option I was like "Huh, why not use it instead?".

CommonJS support is a blocker for the util right now (as I see it), but I feel like nodejs package will need to be replaced at some point, it does not seem to be mantained. I also remember you saying on elixir forums that performance of ssr in development wasn't good, it might be because it currently invalidates require cache on every call, with ESM one would use something like hot loader (I used this one for example) and I'm interested if there will be any difference in speed.

[woutdp]

but I feel like nodejs package will need to be replaced at some point, it does not seem to be mantained. I also remember you saying on elixir forums that performance of ssr in development wasn't good

Definitely. I'm working on a project (https://territoriez.io/) that uses LiveSvelte quite deeply and I've struggled a bit with SSR. For example it seems there's some memory leakage going on. So I've disabled it for now.

#36 should be an all encompassing issue that replaces it with something better.

So I'm invested in fixing it at some point. But also waiting it out just a little until it's the right time to tackle it. Working with it in production is giving me a good feel of what the pain points still are for LiveSvelte.

[vonagam]

Added half-way implementation for getLive. It requires getContext to be passed as an argument to it, so no dependencies. It is to improve typed experience a little and is also future-migration-friendly when it will not need the argument. Util's usage is optional.

// typed basic
import {getContext} from 'svelte'
import {Live} from 'live-svelte'

const live: Live = getContext("live-svelte")

// typed util
import {getContext} from 'svelte'
import {getLive} from 'svelte-live'

const live = getLive(getContext)

[dev-guy]

I like this. Eliminating the Svelte warnings is a big win IMO since the fewer warnings my team has to ignore, the less likely they are to miss the important ones. SSR, among its other issues (speed, scalability, another server-side library), is unstable for me and I submitted an issue about it. I don't understand how it could be working for other LiveSvelte users. I don't see myself using SSR with LiveSvelte because if I didn't require heavy-duty client-side code, I would use vanilla Phoenix.

[woutdp]

SSR definitely needs a rework. It can be slow and seems to have some memory issues.

I'm not 100% convinced of the proposed solution though.

If we can do something like this:

import {getLive} from "live-svelte"
const live = getLive()

I'd be happy to integrate that.

[vonagam]

For that a change of ssr node provider needs to happen first. To one that supports esm since new svelte does not support cjs.

Also would recommend to add js build artifacts to gitignore. When this package will start to import svelte it will increase the size of the js output. I don't think there is much gain from committing those builds, but they do pollute the history and increase size of the repo. Or am I missing something about why they need to be shown?

[woutdp]

Also would recommend to add js build artifacts to gitignore.

That's right, gitignored and removed them.

[vonagam]

Hm. How about this:
New optional options argument for getHooks with type of {prop?: boolean, context?: boolean}. With default behavior matching {prop: true, context: true}. For a person to disable warnings they would need to pass {prop: false} and use only context access. I don't know if there is any downside to providing a context but put an option to disable it for consistency, can remove it and have only simple {prop?: boolean}.

[vonagam]

Nah, scrap this. Back to context only. Made this work:

import {getLive} from "live-svelte"
const live = getLive()

For that added setupLive method that needs to be called in app.js with getContext from svelte. So those lines were added to the template:

import {getContext} from "svelte"
import {getHooks, setupLive} from "live_svelte"
setupLive(getContext)

Could have done it from the start but wanted to avoid usage of global state. But, since svelte itself uses it and there can only be one svelte, should be ok.

[vonagam]

I've updated the description with current state of the PR. Do you have any reservations about current implementation?

[woutdp]

Saw this but not a 100% sure, if we can get rid of manually passing around the context in the setup then we're good to go.

[dev-guy]

vonagam Why doesn't live_svelte do import {getContext} from "svelte" ? Looks like you're avoiding adding a direct dependency between live_svelte and svelte. I'm curious why. Does that throw a runtime error in the browser despite the fact that getContext isn't called until later? You solution here might also be the solution to #71

[vonagam]

I'm curious why.

Svelte 4 does not support cjs and current runtime expects cjs.

if we can get rid of manually passing around the context

I don't think we can get better than this at the moment. It is just 2 lines in the template.

[woutdp]

I think we should replace Node with Bun before we do this. Bun has reached 1.0 and should be compatible. This will avoid the cjs stuff. This should probably be a separate package inside the Elixir ecosystem. Bun is not available as a library, only as an executable. It has support for ffi and it might be possible to call Elixir code from javascript... There's a huge opportunity here if someone feels like picking up this... my spidey sense it tingling, some really cool things could be built from this I feel. Worth to explore.

[alexkornitzer]

Right so I have had a look at this today because Svelte 5 beta has dropped and I think its going to pair even better with LiveView. It could even solve the slot issue as they have been deprecated in favour of fragments if I have understood correctly.

Bun can easily replace esbuild due to this work https://github.com/crbelaus/bun. But getting Bun to work has been a challenge as its build command is not fully compatible with esbuild as such esbuild-plugin-import-glob and esbuild-svelte won't work without patches as typically they make use of features that are not implemented. Furthermore a concept like the nodejs library is required to execute Bun for SSR, as a hack I just used System.cmd.

Getting the Svelte 5 to render a basic component was pretty easy, just had to handle the breaking changes with components not being classes anymore: https://svelte-5-preview.vercel.app/docs/breaking-changes#components-are-no-longer-classes

[Torkan]

Sorry to resurrect this old discussion, but some of the earlier posts were sort of relevant to what I've been experimenting on 😅

With some minor tweaking, I've managed to get both SSR and client-side working with ES-modules (still using nodejs), using esbuild to compile each svelte component/page entry point.

I've also decoupled the es-modules from the LV-hooks (currently live_svelte needs to import all of them when setting up the hooks), instead importing them dynamically by using a data-attribute with the location of the respective js-module.

The same approach should also work fine when Svelte 5 lands.

I can create a repo demonstrating all of this if you're interested @woutdp, and if you like what you see I might be able to set off enough time to create a PR sometime in the not too distant future as well 😅

[woutdp]

@Torkan Awesome work, repo showing this off is useful! Would definitely be up for merging a PR that implements this too :)

[Torkan]

@woutdp: https://github.com/Torkan/live_svelte_ssr

[vonagam]

So, I'm interested in plans regarding this change @woutdp.

Elixir's node package now supports ESM so there should not be a problem with adding a dependency on Svelte.

But it also can work without any dependency - like in the current state of PR - with a single setup call in app.js.

Either way it can be a breaking change or a configurable one.

I see that you have PR for Svelte 5, will it be a major update or a minor? In case it is a major - does it make sense to make this change at the same time then?

[woutdp]

@vonagam
This has been a while since I looked at this. Do you think it'll be possible to do something like:

import {pushEvent} from "live_svelte"

I'd be more in favor of that approach. I feel that's very clean and egonomic to use, no real boilerplate and should take care of the warnings. And I'd be fine with depracating the export let live way of importing this and using the new way as the only method.

Regarding the Svelte 5 PR, I'm fine either way. Either inlcude it in that one or to have another release for this.

Svelte 5 will just be another version update like 0.x
According to https://semver.org/ that means we're still considered unstable and the API may change at any time. At some point I'd want to release 1.0.0 and have live_svelte be "finished" but there's still things I'd want to add before I do that.

[vonagam]

Hm... I don't think import {pushEvent} from "live_svelte" can work. Just taking the example from readme (with comments removed):

<script>
  // a)
  import {getLive} from "live_svelte"
  const live = getLive()

  // b)
  export let live

  export let number = 1
  
  function increase() {
    live.pushEvent("set_number", {number: number + 1}, () => {})
  }
  
  function decrease() {
    live.pushEvent("set_number", {number: number - 1}, () => {})
  }
</script>

<p>The number is {number}</p>
<button on:click={increase}>+</button>
<button on:click={decrease}>-</button>

In both cases through prop or context a user gets access to an instance of a live component with corresponding apis.

But with bare pushEvent - how would function know which instance to use in such callback?

import {pushEvent} from "live_svelte"

function decrease() {
  pushEvent("set_number", {number: number - 1}, () => {})
}