-
Notifications
You must be signed in to change notification settings - Fork 35
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
Existing inline functionality is not reactive #25
Comments
Here is the current workaround pattern:
However it would be nice to not have to manually create and manage that load state store every time this functionality is needed. |
would be a nice API to be able to access this state like |
That would be ideal. Where I've run into blockers while pursuing that is keeping |
@Akolyte01 hmm do you think it's adding much weight though? can't people opt out of it just by not using the |
actually the access pattern will need to be a bit different to access both data & state easily... accessed at second layer something like
|
really just sketching it out here... but borrowing the API concept from I also added another wrapper called anywho, here's what i came up with so far. const createAsyncStateStore = () => writable<'fetching' | 'paused' | 'idle'>('idle');
const storeMap: Record<string, AsyncReadable<unknown>> = {};
function getStore<T>(key: string) {
return storeMap[key] as AsyncReadable<T>;
}
function useQuery<S extends Stores, T>(
key: string,
stores: S,
fn: (values: StoresValues<S>) => Promise<T>,
initialValue?: T
) {
if (getStore(key)) return getStore<T>(key);
const store = constructStore(stores, fn, initialValue);
storeMap[key] = store;
return store;
}
function constructStore<S extends Stores, T>(
stores: S,
fn: (values: StoresValues<S>) => Promise<T>,
initialValue: T
) {
const errorStore = writable();
const dataStore = writable(initialValue);
const fetchStateStore = createAsyncStateStore();
const stateStore = derived(
[errorStore, dataStore, fetchStateStore],
([$error, $data, $fetchState]) => {
if ($data) {
return 'success';
} else if ($error) {
return 'error';
} else if ($fetchState === 'fetching') {
return 'loading';
}
return 'loading';
}
);
const derivedStore = asyncDerived(
stores,
async (values) => {
try {
errorStore.set(null);
fetchStateStore.set('fetching');
const value = await fn(values);
dataStore.set(value);
return value;
} catch (error) {
fetchStateStore.set('idle');
errorStore.set(error);
throw error;
}
},
true,
initialValue
);
return {
data: { subscribe: dataStore.subscribe },
load: derivedStore.load,
reload: derivedStore.reload,
state: { subscribe: stateStore.subscribe },
fetchState: { subscribe: fetchStateStore.subscribe }
} as AsyncReadable<T>;
}
type AsyncState = 'loading' | 'success' | 'error';
type FetchState = 'fetching' | 'idle';
type AsyncReadable<T> = {
data: Readable<T>;
load: () => Promise<T>;
reload?: () => Promise<T>;
state: Readable<AsyncState>;
fetchState: Readable<FetchState>;
}; and consume it like // store.ts
export const usePost = (postId: string, initialValue?: Post) =>
useQuery(
`post.${postId}`,
[currentProfileStore],
async ([$currentProfile]) => {
return Api.getPost($currentProfile.id, postId);
},
initialValue
);
// Page.svelte
{ data, load, reload, state, fetchState } = usePost('123'); |
So I think we're aligned on basic concept, though I think in exploring this you've run into the same issues I have. First being that making the state reactive doesn't work well with existing svelte syntactic sugar. Requiring destructuring makes using these custom stores harder to use and brings them out of line with the established patterns of the vanilla stores. I definitely don't want to make the package harder to use to get this feature in. The second is weight. Here you are creating multiple internal stores and multiple internal subscriptions to power this logic. I ran into similar issue. So if all of the loadable stores include this functionality you'd end up doubling or tripling the amount of stores required to power an app. I think there may be a solution here using a meta programming approach, which would allow for opting into this functionality on a store by store basis. As an example of the general direction I'd like to go in
Although maybe that's overcomplicating things--providing a construction parameter to opt in may be preferable. |
Awesome that we are thinking towards the same direction - definitely agree with your points on all front :) Couple thoughts here:
|
As a heads up this feature is in beta testing at the moment and should be released soon! |
@Akolyte01 already saw :) very excited! |
Okay took longer than expected but 1.0.10 is now out! This addresses this use case with the |
Example:
{#await safeLoad(myStore) then loadedSuccessfully}
This generates a single promise that resolves on the first load of the store.
If that resolves successfully but then the store is reloaded and fails, the template does not have any access to this new state information.
The text was updated successfully, but these errors were encountered: