-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
Alternative API for Each / Else block #7673
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
Comments
I personally never use promises in my component and wrap them in stores. It would look something like this in my app:
<script>
export let id;
$: records = createFetchStore(`/api/records/${id}`, []);
</script>
{#if $records.loading}
<p>Loading...</p>
{:else if $records.error}
<p>{$records.error}</p>
{:else}
{#each $records.data as record}
<div>{record}</div>
{:else}
<p>0 records retrieved.</p>
{/each}
{/if}
<!--Or use it however you like-->
<div class:loading={$records.loading}>
{#each $records.data as record}
<div>{record}</div>
{/each}
</div>
<style>
.loading {
cursor: wait;
opacity: 0.7;
}
</style>
It will also implicitly abort()
the request when the component is unmounted before it finishes or when id
changes (subscribers drop to 0).
Here's a rough implementation for a similar problem #1857 (comment) (I never got around writing a new RFC for promisable
stores, but I love them and never use {#await}
either).
Edit: FWIW, something like this is also an option:
<script>
let records = null;
let loading = true;
onMount(async () => {
const res = await fetch('/api/records');
records = await res.json();
loading = false;
});
</script>
When retrieving data from an API, it is better to represent "loading" with a nullish value (null or undefined) rather than with an empty array
I wouldn't use data at all to represent a loading state. That's what a loading state is for. And you know if the fetch is still fetching or not.
I personally would prefer @Prinzhorn's solution. Use a store to abstract promise states would make your code much cleaner. But back to OP's proposal, I don't see much point adding new block semantics when existing ones can well do the job. <script>
let recordsPromise = new Promise();
onMount(() => {
recordsPromise = fetch('/api/records').then(res => res.json());
});
</script>
{#await recordsPromise}
<p>Loading...</p>
{:then records}
{#if records.length == 0}
<p>0 records retrieved.</p>
{/if}
{#each records as record}
<div>{record}</div>
{/each}
{:catch error}
<p>{error}</p>
{/await} |
Thanks for the suggestions. For For {#if records == null}
<p>Loading...</p>
{:else}
{#each records as record}
<div>{record}</div>
{/each}
{/if} So, the addition is more for convenience instead of adding new functionality that is currently not possible. |
empty block nice to have, like django style https://www.geeksforgeeks.org/for-empty-loop-django-template-tags/ |
Describe the problem
When retrieving data from an API, it is better to represent "loading" with a nullish value (
null
orundefined
) rather than with an empty array, as shown in the onMount tutorial. An empty array would more correctly represent an empty result set after correctly retrieving no data.Describe the proposed solution
Consider implementing something that will work as follows. The
{:empty}
/{:nullish}
sub-blocks are intended to be used in lieu of{:else}
and are more descriptive of their behavior, and thus more intuitive.{:empty}
has the same behavior as{:else}
, and{:nullish}
is rendered when the variable is nullish (or perhaps, when it is falsey, or even perhaps when it is more generally non-iterable).Alternatives considered
Providing an additional sub-block only to handle nullish values, and to retain the use of
{:else}
for an empty set would be easier to implement, and a non-breaking change, but the meaning of{:else}
is less intuitive and probably not best for Svelte in the long term.or another non-breaking possibility could keep the existing 'else' behavior:
Also considered using something like
{:catch}
for the noniterable case but it might be confused with something that would handle an actual error. (Though maybe it is an actual error?) Something descriptive like{:noniterable}
might be a good choice.Importance
would make my life easier
The text was updated successfully, but these errors were encountered: