Replies: 29 comments 12 replies
-
you should move the If you need the values from the await call you can still create In order to make await fetch() becomes var context = getActiveContext()
await fetch()
setActiveContext(context) It would also require transformations in any |
Beta Was this translation helpful? Give feedback.
-
@posva I have addressed this in the issue I think.
I alluded to some problems, that I'd like to emphasis a little more. function useMouse() {
const pos = reactive({ x: 0, y: 0 })
function listener(e) { pos.x = e.x; pos.y = e.y }
document.addEventListener("mousemove", listener)
// never called!
onUnmounted(() => document.removeEventListener("mousemove", listener))
return pos
} Linter can't catch what happens in opaque functions. Adding a function such as |
Beta Was this translation helpful? Give feedback.
-
I think this is a good question to be discussed and making it better. The best way to handle this is that JS gives some kind of feature to inject some function calls between each async call in an async function, but yeah unfortunately there isn't. Just adding my rough idea on this as an option. With the export function createInstanceScope(instance = getCurrentInstance()) {
return (fn, options) => effectScope(()=> {
const prev = getCurrentInstance()
setCurrentInstance(instance)
fn() // error handling
setCurrentInstance(prev)
})
} Wrap each synchronizes parts that create effects with it. Example usage: {
async setup() {
const instance = getCurrentInstance()
const scope = createInstanceScope(instance)
const data = await fetch()
let processed
scope(() => {
processed = useXXX(data)
watch(/*...*/)
onMounted(/*...*/)
})
await fetch()
scope(() => {
/*...*/
})
}
} Or we can update #212 to allow forwarding the returning object, and it can be: {
async setup() {
const instance = getCurrentInstance()
const scope = createInstanceScope(instance)
const data = await fetch()
const { processed, foo } = scope(() => {
processed = useXXX(data)
watch(/*...*/)
const foo = computed(() => processed + 1)
return { processed, foo }
})
return { data, processed, foo }
}
} (Happy to make a formal RFC with a detailed design if people like this idea.) |
Beta Was this translation helpful? Give feedback.
-
I've actually had this in mind too when working on export default {
async setup() {
const id = await getAsyncRef()
watch(id, () => {})
await doAsyncWork()
useOtherHook()
}
} becomes: export default {
async setup() {
+ let __temp
+ const __i = getCurrentInstance()
- const id = await getAsyncRef()
+ const id = (__temp = await getAsyncRef(), setCurrentInstance(__i), __temp)
watch(id, () => {})
await doAsyncWork()
+ setCurrentInstance(__i)
useOtherHook()
+ setCurrentInstance(null)
}
} No code change required on the users' part. |
Beta Was this translation helpful? Give feedback.
-
For I suppose we still need a "manual" fallback strategy because:
Example: async function useSuffixAsync(r: Ref<string>) {
let suffix: string = await fetch(...);
// Boom: computed is called outside setup context, even in <script setup>
return computed(() => r.value + suffix);
} |
Beta Was this translation helpful? Give feedback.
-
Nits about your codegen:
Combine those two fixes and arbitrary user code with await calls in any places: loops, ifs, try-catch, cases... it becomes a nightmare of a codegen. I think you'd better change the codegen to wrapping each await expression with something like |
Beta Was this translation helpful? Give feedback.
-
Makes sense. I don't think I have to time to work on this right now, but it would be helpful to list as many as possible cases that we need to cover so that we can work out a more complete proposal. |
Beta Was this translation helpful? Give feedback.
-
Another solution would be using generators to avoid complex code generation. The problem is that we cannot inspect async function execution progress. That is, we cannot inject For example export default {
async setup() {
const id = await getAsyncRef()
watch(id, () => {})
await doAsyncWork()
useOtherHook()
}
} can be mechanically translated by replacing export default {
*setup() {
const id = yield getAsyncRef()
watch(id, () => {})
yield doAsyncWork()
useOtherHook()
}
} We can handle generator setup like this if (setup instanceof GeneratorFunction) {
setCurrentInstance(ins)
let gen = setup()
let step = gen.next()
while (!step.done) {
setCurrentInstance(ins)
let val = await Promise.resolve(step.value)
step = gen.next(val)
}
setCurrentInstance(null)
} Generator schema also reduces compiled code bloat. It also helps users to extract setup code in |
Beta Was this translation helpful? Give feedback.
-
@HerringtonDarkholme |
Beta Was this translation helpful? Give feedback.
-
wouldnt be the most obvious solution to provide closures in the setup context? export default defineComponent((props, { computed, watch, onMounted }) => {
// ...
}); i know that it was explicitly not done this way for tree shaking, but imho watch/computed/hooks are so essential in most apps, so they would end up in the bundle anyway? edit: nevermind, that doesnt transpose over to |
Beta Was this translation helpful? Give feedback.
-
@jods4 transaltion for async setup() {
const suffix = await useAsyncSuffix().withVueContext()
}
async function useAsyncSuffix() {
const ret = await fetch().withVueContext() // have to call withVueContext here as well
return computed(() => ret.value + 'suffix')
} Users can write generator function for async hook as well. A global solution has to be applied to nested hook call regardless of its mechanism (unless it's algebraic effect). |
Beta Was this translation helpful? Give feedback.
-
Absolutely, that was my point. You can apply then generators solution, that means you use function *setup() {
const ret = yield helper(useSuffixAsync())
} Personally I think it's getting slightly confusing, especially when you don't have static typing (TS). Some "async" functions ( Speaking of codegen... that idea could be extended everywhere if you put a JS loader in your build and use some kind of marker comment/decorator/label in your code, e.g.: // vue-async-context
async function useSuffixAsync(r: Ref<string>) {
// Magically becomes: await vueContext(fetch(...))
let suffix: string = await fetch(...);
return computed(() => r.value + suffix);
} |
Beta Was this translation helpful? Give feedback.
-
What if we simply compile |
Beta Was this translation helpful? Give feedback.
-
It wouldn't solve async composable functions, unless you make them write generators instead, and consumers add the helper when calling them.
Once you look at async composable functions that depend on Vue scope, Chaining async calls async setup() {
// Whatever happens in generator1 will be perfectly fine
await helper(generator1())
// Because of await, here context is lost and generator2 doesn't work
await helper(generator2())
} So you'll need to combine them with some Versioning breakage:
If we stick to await, in that scenario the helper function is added inside the composable function, hence not requiring source change from consumers. |
Beta Was this translation helpful? Give feedback.
-
some experiments: https://gist.github.com/backbone87/9d41a114f4529ca38fb814ee46d20bc0 export default defineComponent(async (props, ctx, { watch, use }) => {
const result = await fetch(/* ... */);
const { el } = await use(installExt, toRefs(props).arg);
return { result, el };
});
const installExt = defineComposable(({ watch, onMount, onUnmount }, arg: Ref<string> | string) => {
const ext = await import(/* some heavy external lib */);
const el = ref<HTMLElement | undefined>(undefined);
onMount(() => el === undefined ? undefined : ext.install(el, unref(arg)));
onUnmount(() => el === undefined ? undefined : ext.uninstall(el, unref(arg)));
if (isRef(arg)) {
watch(() => arg, () => el === undefined ? undefined : ext.update(el, unref(arg)));
}
return { el };
}); edit: |
Beta Was this translation helpful? Give feedback.
-
That is an interesting take on the problem. Vue 3 exposed all its hooks as globally imported functions. To be effective, this idea requires that the only way to get them is passed into parameters by Vue, so the global imports need to be removed. That's a massive API change after release. |
Beta Was this translation helpful? Give feedback.
-
Nothing has to change about existing code. (Look into the gist) |
Beta Was this translation helpful? Give feedback.
-
Then someone might have published a sync You have fragmented the ecosystem into those that are async-compatible and those that aren't. Worse: because global import is much more convenient and existed since v3.0, that's what everyone will do by default -- and not work in async code. Also, there's now two ways to do the same thing, which is always confusing. And not only is it two ways to author a composable, it's also two ways consume it. |
Beta Was this translation helpful? Give feedback.
-
if your lib doesnt support async, thats how it is, i think that popular libs would update fast. my main argument is still:
edit: (no matter if sync or async) edit2: we could add a compat layer: const frobAsync = defineComposable(({ withInstance }) => {
return withInstance(() => useWhatever());
}); edit3:
edit4:
(sorry for all the edits) |
Beta Was this translation helpful? Give feedback.
-
To make my argument clearer, here's a situation that's highly undesirable: import { computed } from "vue";
function useLength(str) {
return computed(() => unref(str).length)
} It's perfectly fine for what it does but notice it uses computed. Maybe you took a dependency on it and use it in one of your components: setup(props) {
return { length: useLength(props.name) }
} Life is good. Suppose you need to refactor your component and want to fetch that value from server first. Your That's error-prone (you won't even notice); and it's highly undesirable that Vue becomes fragmented into sync/async like that: at this point you either can't perform this refactoring (but what if you need to?) or you must find a substitute composable for
That idea is neat, but going down that road just leads to removing the global exports, which is my initial comment about this.
Could you elaborate on how it would work?
A 3rd alternative that's been discussed in this thread is putting the burden on the dev who writes async code (whether a composable lib or your own components).
That example is basically the same. |
Beta Was this translation helpful? Give feedback.
-
maybe i am off track here, but in this case since async setup(props) {
const name = await getName(props.id);
return { length: useLength(name) };
}
const frobAsync = defineComposable(({ withInstance }) => {
return withInstance(() => useWhatever());
});
function createWithInstance(instance) {
return (fn) => {
const parent = getCurrentInstance();
try {
setCurrentInstance(instance);
return fn();
} finally {
setCurrentInstance(parent);
}
};
} edit: this is only required, if you use a composition function that uses component lifecycle without adhereing to the
only if your composition function relies on component lifecycle, others can be the same as they are now. |
Beta Was this translation helpful? Give feedback.
-
No it doesn't. After EDIT: to clarify,
This works only if you call this before the first await, store a callback to your composable, which you'll use after the await.
Well, yes. That's exactly the case we're discussing, aren't we? I noted that if we keep both the global exports and the parameter-provided hooks, people are likely to continue using the global ones as they are easier to use. |
Beta Was this translation helpful? Give feedback.
-
my bad then, i thought computed is independant of the component lifecycle. i still dont understand why it needs to be dependant on it, because its dependencies should just cleanup themselves on unmount, if set up correctly (read: set up with instance != null). however, if computed needs the instance as well, then it needs to be exposed as a closure just like lifecycle hooks, use & withInstance. edit: yeah, ok computed is based on effect which needs to be stopped. for some reason i thought it would be enough, if it gets garbage collected. edit2: i am pretty sure since computeds should be side effect free, that they could be tracked weakly, but this may be a browser support issue?
i dont understand this really. have you checked the gist? withInstance will be passed through from the setup function: async setup(props, ctx, { withInstance, use }) {
await fetch();
withInstance(() => useFoo());
const baz = use(bar, toRefs(props).arg);
return { baz };
}
const bar = defineComposable(async ({ withInstance }, arg: string) => {
await fetch();
const baz = withInstance(() => useFoo());
return { baz };
}); in this example all uses of foo & bar have the correct instance set.
for sync composition this is no problem. changing a sync composition function to async is by definition a BC break, so this is also not an angle for problems. |
Beta Was this translation helpful? Give feedback.
-
It's off-topic but briefly: the underlying primitive is actually
OK I see and got mislead by I'm still gonna argue EDIT: and I said it before as well but another argument in favor of the former is that it could be automated by a build-time transformation. It's impossible for the latter.
Sync composition functions are impacted, see |
Beta Was this translation helpful? Give feedback.
-
adding dependency to component lifecycle is a BC break. its just hidden by the fact that v1 doesnt use a global variable and v2 does.
sync composables that dont use the component lifecycle will work without offtopic:
yes i checked it, but effects are strong references in the tracking system, which is needed for watch/watchEffect, where no other handle is held (most of the time). but computeds have a strong reference held by the component anyway and hold a reference to their effect, so the tracking system may hold only weak references to computed effects and therefore computeds may not need to be cleaned up. |
Beta Was this translation helpful? Give feedback.
-
Is there any official Vue recommandation that says this? I'm not aware of it and I'm pretty sure if we poll the community, most would say adding a Take the example I gave you into the context of a very large enterprise project and it makes evolving composables very hard. It's an undesirable characteristic and it doesn't have to be that way. The other proposal I pointed out doesn't have this flaw. |
Beta Was this translation helpful? Give feedback.
-
that has nothing to do with vue. global variables are part of your functions contract (just like the external modules you may use). that is why global shared (mutable) variables are considered a code smell, because they hide dependencies. just consider this overexagerated counter example: let a = 0;
let b = 0;
function sumThatWillNeverBreak(...args: any[]): any {
return a + b;
}
// lets use our sum utility:
a = 20;
b = 22;
const c = sumThatWillNeverBreak(); do you think that |
Beta Was this translation helpful? Give feedback.
-
@backbone87 Sorry I don't see the link, mutable variables != functions. |
Beta Was this translation helpful? Give feedback.
-
I just implemented support for <script setup>
await foo()
</script> is compiled to: import { withAsyncContext } from 'vue'
export default {
async setup() {
await withAsyncContext(foo())
// current instance perserved
}
} Where export async function withAsyncContext<T>(
awaitable: T | Promise<T>
): Promise<T> {
const ctx = getCurrentInstance()
if (__DEV__ && !ctx) {
warn(`withAsyncContext() called when there is no active context instance.`)
}
const res = await awaitable
setCurrentInstance(ctx)
return res
} In |
Beta Was this translation helpful? Give feedback.
-
Async setup is a great addition to the composition API.
It enables doing async work (typically fetching resources or data) from the network before the component is displayed and this useful idea is found in other frameworks, e.g. lifecycle
activate
can be async in Aurelia.The problem
It has a flaw though: some primitives such as
watch
,watchEffect
orcomputed
want to know in the context of which component they are called.They will be automatically stopped when the component is unmounted, which cleans up their dependencies.
The problem is that after an
await
, or aPromise.then
, the continuation doesn't run as part ofsetup()
, which has already returned. So there's no component anymore:watch
(and co.) think they run globally and won't stop automatically.Vue linter plugin warns against that with
vue/no-watch-after-await
.🐛 Linter bug: it doesn't warn me about
computed
usage.The solutions
Ignore it
watch
and co. actually still work. The issue hangs on whether their dependencies have a longer lifetime than the watch itself.If you only watch stuff inside the component, then when the component is unmounted everything will happily be GC'ed, no need to do anything.
This is a dangerous game because if you watch a global dependency that outlives the watch, then you have a memory leak and potential unwanted side-effects.
Any function call may hide dependencies, so in complex codebases it's not something you can easily be sure of just by looking at your watch in isolation 👎
You could stop them manually in unmount. 😕
Move your code
You can run your watches in the
mounted
hook, that works and for all purposes seems as good as setup.computed
not as much. You return it from setup, so your only option is to move it before the firstawait
call.This will create a cascade effect, as your computed depends on state that most likely comes from your async call.
So you move the declaration of all state as well, and possibly turn static non-reactive state into refs because they'll change when the async call completes.
This is highly annoying as it makes you declare things twice, create refs where you don't need them and if using TS declare the type of variables that might have been inferred. The promise of
async
is to write code as simple as sync code, while having async calls in the middle, and to me that is not achieved if I must put all my async code at the end.I have large async components and it's really not a joy to write code in this way.
This will also break down when calling composable functions. Any
useXYZ()
is likely to usewatch
orcomputed
internally and so must be moved before the firstawait
.Some composable functions may expect to receive some specific values as parameters when they are called -- if that value would come from the async call you can't use that composable, which would need to be modified to accept a
ref
parameter instead.Notice: the linter is unable to warn users about this. It will happen for sure.
If we write code this way, we might as well give up on the added complexity of
async setup
andSuspense
.What I described here can be done in a normal
setup
and the template of my component can include a generic "loading" component that is bound to the final promise in my setup.Use bound watch
You could create a
watch
that is bound to the current component and then use this function afterawait
.It works in setup but it has to be done for each such function:
watch
,watchEffect
,computed
and it's a bit ugly code.It fails when calling composable functions as they would call the global Vue
watch
. A good solution should solve this issue globally.Flow an async context
This is how .NET for example solve similar issues: the runtime is able to flow some contextual data across async continuations.
Sadly AFAIK there's no such thing in JS.
The closest emulation we could provide is function that "flows" that context explicitely.
You can bikeshed the name. Also I have written this as if I had added
withVueContext
to the prototype ofPromise
.If you don't like that you can change the syntax to
await withVueContext(ajaxCall())
, or in the future maybeajaxCall() |> withVueContext
if the pipe operator becomes a thing.This is the best idea I have, it enables writing natural async code and works in all circumstances (incl. opaque composable functions).
One must not forget the call, but that could easily be enforced by the linter.
Other ideas?
Beta Was this translation helpful? Give feedback.
All reactions