sveltejs / svelte

Cybernetically enhanced web apps
https://svelte.dev
MIT License
78.45k stars 4.11k forks source link

Allow $effect.pre to run on the server #9278

Open Thiagolino8 opened 11 months ago

Thiagolino8 commented 11 months ago

Describe the problem

A common practice when using SvelteKit is to use reactive expressions for expressions that need to be executed reactively on the client but also need to be executed non-reactively on the server during ssr Svelte 5 will bring the $effect and $effect.pre runes to replace reactive expressions, but with the difference that these runes will not be executed on the server, creating the need to repeat code to achieve the same behavior

// before
$: {
    // really big expression
}
// after
import {browser} from '$app/environment'

if(!browser) {
    // really big expression
}

$effect(() => {
    // really big expression again
})

Describe the proposed solution

The reason the $effect rune does not run on the server is because it runs after the DOM is mounted, but the $effect.pre rune does not need to wait for the DOM, and is therefore ideal for reproducing the behavior of reactive expressions Just as the $state and $derived runes are transformed into common variables during SSR, the $effect.pre rune can be transformed into a simple IIFE

Alternatives considered

Manually transform the expression into a function, but this requires changing the code design and still requires duplicate execution with environment checking

Importance

would make my life easier

Conduitry commented 11 months ago

The recommended way to do this currently is to put // really big expression inside a function and to call it both inside if (!browser) { } and to pass it to $effect( ). You could wrap this whole thing up in a helper function if you found yourself doing it a lot. I don't think we want to make $effect.pre run on the server as well. The example in the docs demonstrates a very DOM-centric usage of the rune.

benmccann commented 10 months ago

One idea from 7nik on Discord:

$effect.pre(() => { ... }, { server: true });
aradalvand commented 10 months ago

Rich hit this very issue (or rather, a problem that would've been solved if $effect.pre ran during SSR as well) today in the middle of this livestream.

I second that $effect.pre very much seems like it would/should run on the server too. I'm still not sure I understand the rationale behind the resistance to that. This would be desirable/needed in quite a big portion of the scenarios in which this rune is likely to be used (the one in the video linked above being one basic example).

"Why doesn't $effect.pre run on the server?` is also a question that has come up a few times on the Discord server, suggesting that it's something people generally expect.

Rich-Harris commented 5 months ago

Can someone give me an example of work you'd want to do in an $effect.pre that should happen in both places? I've found it hard enough to find a legitimate use case for $effect.pre at all, let alone one that makes sense during SSR.

The replacement for $: x = ... is const x = $derived(...), not $effect.pre(() => x = ...), and $derived runs during SSR. The example from that livestream is really just an example of a case where we need to get rid of stores — we're mixing and matching paradigms and having to write bad code as a result. It's true that some version of that situation will arise in other apps, but that's a reason to migrate away from stores, not to add the wrong primitives.

Thiagolino8 commented 5 months ago

The livestream example is a great

And even though it can be fixed by turning spring and tweened into utilities that return reactive values, many third-party solutions (like tanstack query) make use of stores or the store contract

Another good reason is the reactive use of context (also used in tanstack query) which cannot be used within $derived as it needs to be executed during component initialization

I also tried migrating the beforeUpdate example from the tutorial to runes and it only works with $effect.pre, it doesn't work with $derived, not sure if it's a bug

Rich-Harris commented 5 months ago

If you're referring to the autoscroll example, it's not a great example to begin with: https://github.com/sveltejs/svelte/issues/9248#issuecomment-2026268610

which cannot be used within $derived as it needs to be executed during component initialization

This is somewhere you'd use $derived.by rather than $derived - grab the context on init and return a function that reads its properties

Thiagolino8 commented 5 months ago

Most effects should be event handlers

The scroll example is not a good example because the state is only modified in one place, it is very simple to move the side effect logic into the event handler

But if a state is modified in several places it doesn't make sense to add the "side effect" in all of them

Having a point where you can react to state changes indiscriminately is a much more common use of effect than a point where it is safe to reference the DOM or perform cleanups

Even React recommends running side effects during render to react to a state change when an event handler is not possible/practical and waiting for the DOM would be unnecessary/harmful, $effect.pre is the closest equivalent to that

function List({ items }) {
  const [isReverse, setIsReverse] = useState(false);
  const [selection, setSelection] = useState(null);

  // Better: Adjust the state while rendering
  const [prevItems, setPrevItems] = useState(items);
  if (items !== prevItems) {
    setPrevItems(items);
    setSelection(null);
  }
  // ...
}
Thiagolino8 commented 5 months ago

This is somewhere you'd use $derived.by rather than $derived - grab the context on init and return a function that reads its properties

As I said some 3rd party libraries use the context internally, manually retrieving the context in a separate location is unfortunately not always an option, which is why $effect.pre is more ideal

peterkogo commented 5 months ago

Can someone give me an example of work you'd want to do in an $effect.pre that should happen in both places? I've found it hard enough to find a legitimate use case for $effect.pre at all, let alone one that makes sense during SSR.

In Svelte Flow you can pass a lot of configuration options as props to our main component. To prevent prop drilling and for architectural reasons (interactive flow graphs cannot be solved with simple top-to-bottom state flow), we are syncing these props to an internal store.

Before, we could just do the good ol'

$: syncState(some, props)

Now, to fix SSR (and hydration mismatches) we have to do:

syncState(some, props) // initial run on server
$effect.pre(() => {  syncState(some, props) });

This is not ideal because it will initially run twice on the client. (Not too bad, but also not very good...) To prevent this, we cannot make the first call conditional, because we don't know in which environment someone will use our library in, so no if (browser) syncState() possible unfortunately.

To make the $effect.pre skip its initial run we could introduce some additional state that would be checked each time something changes.

Bottom line: This issue is not a showstopper at all but things would be simpler if effect.pre would just run on the server.

P.S. $effect.pre is quite useful to have because we want to sync our store before rendering the next state.

peterkogo commented 4 months ago

Having worked around this issue quite a bit more, I would like to double down on it.

I'd argue that $effect.pre is similar to useLayoutEffect in React. The goal is to process some side effects before the next render, which should also include the initial server side render.

In accordance with your comment @Rich-Harris about this issue being solved with a $derived state, the only reliable solution I have found is to use

let useless = $derived.by(() => { syncToGlobalState(some, props); });
useless; // needed, so the compiler does not throw it away

I understand the aversion of complicating things in regards to the $effect hook, but then maybe an addition to the $derived rune, which does not return a new state, is in order? [Edit: I was mistaken, this does not work]

Again, this is an edge case. I can live with these complications for the greater good and keeping things simple, however I am pretty sure this issue will arise for any library maintainers requiring some kind of global state.

I welcome any solutions, of course, maybe there is a possibility I haven't found yet.

aradalvand commented 4 months ago

$effect.pre not running on the server continues to make zero sense.

tysonclugg commented 1 month ago

@Rich-Harris asked:

Can someone give me an example of work you'd want to do in an $effect.pre that should happen in both places? I've found it hard enough to find a legitimate use case for $effect.pre at all, let alone one that makes sense during SSR.

One use case is building prerendered static pages with export const csr = false.

I'd be happy if any of the following resulted with <body data-theme="foo" ... in my prerendered static site with CSR disabled:

I was quite surprised when none of the above had any effect with CSR disabled.

7nik commented 1 month ago

I'd be happy if any of the following resulted with <body data-theme="foo" ... in my prerendered static site with CSR disabled:

* `$effect.pre(()=>{if(document?.documentElement) document.documentElement.dataset.theme=pageTheme});`

* `<svelte:document use:setPageTheme} />`

* `<svelte:document data-theme={pageTheme} />`

Obviously, the first option can work only in the browser environment and only proves that $effect.pre shouldn't run on the server. The second option is an action — they run only on the client and require CSR. The third option makes no sense — document isn't an HTML element, and I am even surprised that it doesn't emit an error or warning.

Basically, your issue is https://github.com/sveltejs/svelte/issues/3105 and related ones. And the workaround is probably still transformPage.