Replies: 17 comments 5 replies
-
I see. That's interesting. I guess with that API it doesn't even need to be a function accessor. It just has 2 getters. My instant hesitation was I liked the whole feels like a Signal thing.. with Suspense you rarely use the loading or error states. We can't do that union trick the other way I guess. Like this won't fly? interface Pending {
loading: true;
error: false;
(): undefined;
}
interface Success<T> {
loading: false;
error: false;
(): T;
}
interface Failure {
loading: false;
error: any;
(): undefined;
} I'm gathering not or you would have suggested it. Hmm, it is breaking though which ties my hands I think for now. |
Beta Was this translation helpful? Give feedback.
-
I think that should work as well! Although I'm not sure if we can correctly eliminate the types (for instance, checking for |
Beta Was this translation helpful? Give feedback.
-
Yeah this shows a lot of my misunderstanding of TS I was thinking keeping things in their own lanes would help typing instead of pushing the error through data. As I was saying loading and error were sort of secondary considerations. I don't want data to hold the error incase people aren't checking for it. I think status is probably fine addition and wouldn't be breaking. |
Beta Was this translation helpful? Give feedback.
-
I added a TypeScript playground link to demonstrate type eliminations with your idea (+ |
Beta Was this translation helpful? Give feedback.
-
Yeah we can do that I need to figure out which state is the source of truth.. status is probably derived like {
get status() {
return error() ? "failure" : loading() ? "pending" : "success"
}
} Question of which strings we should use. Is it "success"? is it "failure"? Sometimes resources will start at "success" I guess. . Is there a 4th state when it's never run and isn't triggered initially? |
Beta Was this translation helpful? Give feedback.
-
Edit: const [status, setStatus] = createSignal('pending');
// ...
setStatus('pending');
fetcher().then(
(value) => {
if (notDisposed) {
setStatus('success');
}
},
(value) => {
if (notDisposed) {
setStatus('failure');
}
},
); |
Beta Was this translation helpful? Give feedback.
-
Ok yeah because it is just boolean not true. And that's fine because status enough to type narrow ok. Yeah I am onboard for this then. Let's see if anyone has any thoughts. But I think we can get this out in the next minor version I guess. |
Beta Was this translation helpful? Give feedback.
-
After re-evaluating, I think there is a possible 4th state: |
Beta Was this translation helpful? Give feedback.
-
I think this proposal is fine, though I'm not sure about "stale". Before moving forward with it, I think we should examine some realistic use-cases to see how well it works in practice. Switching on a string value using a real JS function Comp() {
const [data] = createResource(...);
return (
<Switch>
<Match when={data.status === 'pending'}>
<h3>Loading...</h3>
</Match>
<Match when={data.status === 'failure'}>
<h3>{data.error}</h3>
</Match>
<Match when={true}>
<div>{data()}</div>
</Match>
</Switch>
);
} While not a reason to do nothing here, when using the control flow components, type narrowing doesn't really work anyways thought I'm hopeful we may be able to do more with TypeScript 4.4. Should probably try out the beta. |
Beta Was this translation helpful? Give feedback.
-
There's not much in 4.4 that can help us narrow types here. |
Beta Was this translation helpful? Give feedback.
-
That works but it also changes the behavior. It basically keys it since data being passed through can change. Not just boolean. So type narrowing is just hard. Because our control flow isn't even keywords. Im not even sure LSP hacks make sense. |
Beta Was this translation helpful? Give feedback.
-
I looked at React query they have 4 states as well. "idle", "loading", "success", "error". They consider it idle whenever it is in a don't fetch state ie.. if the query is null in our case. They don't have "stale" but also have a "isFetching". To be fair I use "loading" this way and it's up to the user whether they unmount it. But it appears React query only gives the "loading" status when there is no data available. I'm not sure there needs to be a no data check other than checking for data yourself. It is interesting because changing the query in Solid doesn't wipe the resource value. We keep it around so I realize our proposed types aren't quite right. We can use "stale" for that I guess. But it also doesn't remove from the "success" state in a sense. So there is a question of whether we cater status for this initial load story because that is not how I've node the independent states. |
Beta Was this translation helpful? Give feedback.
-
Meanwhile, |
Beta Was this translation helpful? Give feedback.
-
Here's a flowchart.
|
Beta Was this translation helpful? Give feedback.
-
Yeah I think it's interesting because React Query's
The main case for idle is fetching on demand. Like when the component loads you don't load it. But then on click load data. It isn't treated like |
Beta Was this translation helpful? Give feedback.
-
Is there an example case-scenario for this? I do think this works well for call-to-action fetches (e.g. POST) and not for data-derived UI. I may be wrong tho. Edit: |
Beta Was this translation helpful? Give feedback.
-
Hi everyone, I have been thinking about asynchronous rendering and the API surrounds it. I tried to formalize my findings and ideas in an RFC like format. Lets start with the data to be tracked inside Data To Track Inside
|
Beta Was this translation helpful? Give feedback.
-
Currently,
data
returns an accessor that returns the current result of the resource. To check if it is still loading, we usedata.loading
and usedata.error
when checking for failures. This is great but not for Typescript.https://github.com/solidjs/solid/blob/main/packages/solid/src/reactive/signal.ts#L216-L233
The accessor returns
T | undefined
but that is incorrect for 3 reasons:Suspense
should guarantee the type is exactly what is returned fordata
.undefined
is a valid value for a Promise to result into.data.loading
anddata.error
is not exclusive enough to perform type elimination.There is a way to fix this but would introduce a breaking change:
This way, the result is deterministic, exclusive and can be type-eliminated.
edit:
Playground
Beta Was this translation helpful? Give feedback.
All reactions