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

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

Open
Thiagolino8 opened this issue Sep 30, 2023 · 11 comments
Open

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

Thiagolino8 opened this issue Sep 30, 2023 · 11 comments
Labels

Comments

@Thiagolino8
Copy link

Thiagolino8 commented Sep 30, 2023

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
Copy link
Member

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.

@dominikg dominikg added the runes label Oct 2, 2023
@benmccann
Copy link
Member

One idea from 7nik on Discord:

$effect.pre(() => { ... }, { server: true });

@aradalvand
Copy link

aradalvand commented Nov 17, 2023

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
Copy link
Member

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
Copy link
Author

Thiagolino8 commented Apr 4, 2024

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
Copy link
Member

If you're referring to the autoscroll example, it's not a great example to begin with: #9248 (comment)

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
Copy link
Author

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
Copy link
Author

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
Copy link

peterkogo commented Apr 23, 2024

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
Copy link

peterkogo commented Apr 26, 2024

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
Copy link

aradalvand commented Apr 26, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants