svelte icon indicating copy to clipboard operation
svelte copied to clipboard

'$' accessors subscribe to store when conditionally accessed instead of upon component mount

Open Akolyte01 opened this issue 2 years ago • 8 comments

Describe the problem

Forgive me if this is a duplicate.

Currently store values accessed with '$' syntax in templates are subscribed to when the corresponding component mounts, regardless of that accessor's placement within Logic (if/else or await) blocks. For example:

<script>
  const someData = readable('', (set) => { fetchSomeData().then(set) });
  let displaySomeData = false;
</script>

<button on:click={() => displaySomeData = true}>fetch some data</button>
{#if displaySomeData}
  {$someData}
{/if}

Given an instance of this component, fetchSomeData is not called when the button is clicked. Instead, when the component mounts a subscription is made to someData and so fetchSomeData is called at that time.

This behavior makes sense when considering how component templates are compiled, but is unintuitive both for new and experienced Svelte developers. When dealing with asynchronous data this pattern makes it difficult to properly lazily load data and can easily result in bugs and regressions.

For example we developed a new feature that fetched data from a new endpoint (in a store to allow for easy hydration of relevant components). Access to the feature was gated behind a feature flag that could be turned on when our data was ready, allowing us to enable the feature independently in our staging and production environments. We were aware of this peculiarity of svelte and made efforts to prevent the store from being subscribed to prematurely, but all it took was a single place where this was not accounted for; the store was subscribed to prematurely, causing us to hit the endpoint before it was ready, resulting in exceptions that broke a portion of our app in production (where our data was not yet ready) despite the feature flag being off.

Describe the proposed solution

Svelte should reactively subscribe to a store only when a corresponding $ accessor is active in a given Logic block. In the component example given in the previous example, this would mean someData would be subscribed to when displaySomeData is true, and unsubscribed from when displaySomeData is false.

Alternatives considered

If a developer is aware and cognizant of this behavior in Svelte there are multiple ways to circumvent or mitigate this issue.

One solution would be to not include lazily loaded asynchronous behavior in a readable stores start function, and to instead manually perform that async behavior when desired and write the result to a writable store.

Another solution is to put the store accessor into a separate component, and then to conditionally render that component. Components mount when conditionally rendered and so the child component will not subscribe to the store until this happens. i.e.

Child.svelte

<script>
  const someData = readable('', (set) => { fetchSomeData().then(set) });
</script>

{$someData}

Parent.svelte

<button on:click={() => displaySomeData = true}>fetch some data {#if displaySomeData} <Child/> {/if}


When we first became aware of Svelte working in this manner we decided to employ the second example. This works very well in some scenarios, but as mentioned earlier is very easy to miss by both new and experienced developers alike. 

### Importance

would make my life easier

Akolyte01 avatar Dec 14 '22 17:12 Akolyte01

I personally consider the current behavior correct. I don't think the proposed solution fixes anything, it just takes the issue and moves it somewhere else. E.g. what if displaySomeData can be toggled (or bound to a collapsible like <details bind:open={displaySomeData}>). Now people will complain that the store keeps getting sub/unsub every time. Or what if I don't want fetchSomeData() to be lazy? I want the data to be fetched ASAP so that when displaySomeData is toggled it is usually already there.

Currently it is trivial to know what's happening at any point. You have a store, it is subscribed. Done. With the proposed solution and the possibility of multiple conditional $someData in a single component it becomes increasingly difficult to know what the component does.

We were aware of this peculiarity of svelte and made efforts to prevent the store from being subscribed to prematurely, but all it took was a single place where this was not accounted for; the store was subscribed to prematurely, causing us to hit the endpoint before it was ready, resulting in exceptions that broke a portion of our app in production (where our data was not yet ready) despite the feature flag being off.

That does sound like a broken architecture on your end (too much responsibility on the client), but that's just from the little information provided.

Prinzhorn avatar Dec 14 '22 18:12 Prinzhorn

You have a store, it is subscribed. Done.

But that's not what happens. You have a store, it is subscribed when the component mounts. With the current implementation if you take a conditionally rendered section of a template and move it to a child component you suddenly get very different behavior in how the store is accessed. This change would bring store subscriptions in line with how the rest of Svelte works with conditionally rendered components.

The problem of the start function being called multiple times is also not avoided by the current solution because of child components mounting and unmounting. Making async behavior 'call once' is it's own consideration.

in a single component it becomes increasingly difficult to know what the component does.

Not really? It is exactly as difficult as knowing how the Logic blocks resolve. There is not any new complexity added here.

what if I don't want fetchSomeData() to be lazy?

Yes, this admittedly requires an additional step. However that step, subscribing in the components script block, is far easier to accomplish than enforcing lazy loading when the default behavior is not. Accidentally lazy loading is also easier to notice and likely to result in bugs than the other way around. This becomes increasingly complicated when dealing with derived stores.

That does sound like a broken architecture on your end (too much responsibility on the client),

Deploying in progress features behind a flag is a very common practice for large/complex web apps. It allows you to keep parity between staging and production environments.

Akolyte01 avatar Dec 14 '22 18:12 Akolyte01

Deploying in progress features behind a flag is a very common practice for large/complex web apps. It allows you to keep parity between staging and production environments.

I'm aware, I was just completely misinterpreting "causing us to hit the endpoint before it was ready" interpreting "ready" in a timely sense, not in the sense of it existing.

I won't comment any further on the rest, let's wait for a Svelte member to comment.

Prinzhorn avatar Dec 14 '22 18:12 Prinzhorn

I think the best way to deal with this is to conditionally create the store, based on the feature flag.

One possibility is something like the below REPL, which puts the feature flag check and store creation into a single function, and calls it reactively as the feature flag changes. If the flag is disabled, it returns a store that does nothing and just contains an empty value.

In a real app the function that creates the store would probably be in a separate file and everything that needs to use it would call that function, and pass in the feature flag (or the user object containing the flags, or whatever) to ensure that nothing forgets the check.

https://svelte.dev/repl/ee6943fb7e294b4ebc1b01771025b120?version=3.55.0

dimfeld avatar Dec 15 '22 08:12 dimfeld

I think the best way to deal with this is to conditionally create the store, based on the feature flag.

One possibility is something like the below REPL, which puts the feature flag check and store creation into a single function, and calls it reactively as the feature flag changes. If the flag is disabled, it returns a store that does nothing and just contains an empty value.

In a real app the function that creates the store would probably be in a separate file and everything that needs to use it would call that function, and pass in the feature flag (or the user object containing the flags, or whatever) to ensure that nothing forgets the check.

https://svelte.dev/repl/ee6943fb7e294b4ebc1b01771025b120?version=3.55.0

This solution doesn't scale very well when the feature intersects with large portions of the app. A similar approach that scales better would be to have the store itself access the feature flag and to short circuit to some default value if the flag is not turned on.

But yes, there are multitude of ways to achieve the desired behavior. However the default behavior is still unintuitive, prone to causing bugs, and, critically, works unlike anything else in svelte!

Functions invocations, component declarations, etc, do not evaluate or 'run' when included inside Logic blocks until rendered. As an example here's a rewrite of my initial example using promises https://svelte.dev/repl/d08db2ecd7d44eff8600abf69ce8e5d5?version=3.55.0

fetchSomeData is only called when displaySomeData evaluates to true.

Akolyte01 avatar Dec 15 '22 17:12 Akolyte01

This problem also crops up in other places, not just when dealing with async data. https://svelte.dev/repl/71951e1141f649958abfb0ee317e025b?version=3.55.0 Here we see that logic that would otherwise work breaks your app when moved to a store because of the 'premature' subscription. Obviously this is a very simple and contrived example with an easy solution, but the general principle applies to more intractable scenarios.

Akolyte01 avatar Dec 15 '22 17:12 Akolyte01

This solution doesn't scale very well when the feature intersects with large portions of the app. A similar approach that scales better would be to have the store itself access the feature flag and to short circuit to some default value if the flag is not turned on.

Yes, it was just an example designed to demonstrate a concept and I have no idea how your app manages feature flags. No need for nitpicking.

TBH I think the proposed solution has its own set of problems. To what length should the compiler go to determine whether a store should be subscribed or not? What if a store is referenced in a reactive block but in a code path that hasn't run yet? How about in a ternary expression? Does every reference to a store, even in the JavaScript code, need to compile to some kind of get-or-create that subscribes if needed? That's possible, sure, but it's also a fair amount of extra complexity, especially if you want to make it performant when using store values in hot paths.

And if the compiler doesn't handle these cases like it does in a template, then what happens if the user moves the reference to the store from the template into a reactive statement, but in a code path that isn't running. It's the same problem again where things suddenly break.

Then, what if the user has the opposite case, and actually does want the store to run (e.g. to fetch data) before the template uses it. Do they use a reactive statement that just does $: value = $store just to get the store to run? That's not a great experience either.

dimfeld avatar Dec 15 '22 21:12 dimfeld

TBH I think the proposed solution has its own set of problems. To what length should the compiler go to determine whether a store should be subscribed or not? What if a store is referenced in a reactive block but in a code path that hasn't run yet? How about in a ternary expression? Does every reference to a store, even in the JavaScript code, need to compile to some kind of get-or-create that subscribes if needed? That's possible, sure, but it's also a fair amount of extra complexity, especially if you want to make it performant when using store values in hot paths.

I don't think I see the same problem with determining when the store should be subscribed to. A new subscription would be created under the exact same circumstances that a function invocation would run. That is already handled by the compiler. But yes, this would not be a trivial change. It would require additional book keeping in the compiled code. As for performance, I don't know enough about how things work under-the-hood to say. It's a possibility, but I can't see how it would be any less performative than how function invocation in logic blocks already work.

And if the compiler doesn't handle these cases like it does in a template, then what happens if the user moves the reference to the store from the template into a reactive statement, but in a code path that isn't running. It's the same problem again where things suddenly break.

I'm not sure I follow this example.

Then, what if the user has the opposite case, and actually does want the store to run (e.g. to fetch data) before the template uses it. Do they use a reactive statement that just does $: value = $store just to get the store to run? That's not a great experience either.

I really don't think this is a problem. Or at least, not a new one. As mentioned above you already need to account for this if you have a store that is used in a conditionally rendered component. If you want the store's start function to run before the store is mounted you need to create the first subscription somewhere else.

Akolyte01 avatar Dec 15 '22 23:12 Akolyte01

The point is that <script> part is executing from top to bottom when the Svelte component is initialized. If the execution order in <script> depends on ${if} statement, it will be super difficult to understand the execution order and it has a lot of chance of making a bug.

So to manage the timing of store initialization is a userland task, not a Svelte task.

baseballyama avatar Feb 26 '23 07:02 baseballyama

@baseballyama I think you may have misunderstood what is being asked. I am referring to subscriptions that occurs in the template with the $ operator, NOT the

Nothing else in svelte works the way store subscriptions in templates do. If that does not result in bugs, why would this?

Akolyte01 avatar Feb 26 '23 17:02 Akolyte01

"manage the timing of store initialization is a userland task, not a Svelte task."

But that's just it. With the current implementation the user CANT control when a store is first subscribed to if they use the $ accessor.

Akolyte01 avatar Feb 26 '23 20:02 Akolyte01

This would increase the compiled output and implementation complexity of the compiler for an edge case which can easily be worked around by either manually subscribing or moving the store+fetch into a sub component. Furthermore the current model is more consistent, at least to me, because intuitively a variable I can use at any place in the component needs to be created at the top level and be always present, not depending on some condition.

dummdidumm avatar Feb 26 '23 21:02 dummdidumm

@dummdidumm You are claiming this adds to much complexity to do automatically, while also claiming this can be easily worked around. That seems like a contradiction to me.

because intuitively a variable I can use at any place in the component needs to be created at the top level and be always present, not depending on some condition.

The variable would still be defined in the same place that it is currently! The thing that would change is where subscribe and unsubscribe would be invoked. Which, again, would make this accessor MORE consistent with the way every other part of svelte works.

And for something that is an "edge case" nearly every new svelte developer I have worked with ends up running into this at one point or another. The current implementation has actively resulted in hard-to-anticipate bugs which have brought down production apps.

Akolyte01 avatar Feb 27 '23 21:02 Akolyte01