Skip to content
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

template ref can not get exposed value from a top level await component #4930

Open
edison1105 opened this issue Nov 10, 2021 · 9 comments · May be fixed by #12082
Open

template ref can not get exposed value from a top level await component #4930

edison1105 opened this issue Nov 10, 2021 · 9 comments · May be fixed by #12082
Labels
has workaround A workaround has been found to avoid the problem 🐞 bug Something isn't working

Comments

@edison1105
Copy link
Member

Version

3.2.21

Reproduction link

sfc.vuejs.org/

Steps to reproduce

click the button

What is expected?

without error

What is actually happening?

got an error

@edison1105
Copy link
Member Author

edison1105 commented Nov 10, 2021

see #4891 (comment)
the component is not resolved when setRef.
image

@edison1105
Copy link
Member Author

as a workaround, from @sodatea
defineExpose should run before the first await.
see demo

@languanghao
Copy link

@edison1105 I tried that, but I do not think it's a good workaround.

As code below, expose method need invoke a var returned by a async method.

const props = defineProps<id: string>()

const recorder = await getRecorder(props.id)
const save = () => {
    // post recorder to server
}
defineExpose({ record })

If I move defineExpose({ record }) to the front of async function, then I have to change recorder from const to var.

这种写法存在代码上的逻辑撕裂,如果可能的话请加以增强。

@LinusBorg
Copy link
Member

LinusBorg commented Nov 11, 2021

Currently, for template refs Vue relies on the presence of instance.exposed to decide wether this is a closed instance – requiring an exposeProxy– or a normal, open instance which exposes the normal instance.proxy.

This is usually fine, but becomes a problem with an async component like the one in this issue, because here, instance.exposed will be set after the instance was assigned to the parent's template ref, so the parent''s ref doesn't point to the exposeProxy.

So what we would need is a way to recognize that a component was created with script setup and is therefore closed by default right when the component instance is being created, not only once exposed() has been called, maybe a option flag we sneak in during compilation.

But that would still leave us with a challenge: While the async component is still pending, instance.exposeProxy would exist, but still be unaware of the sayHi function as expose() hasn't been called yet. That would mean that comp.sayHi would still be unavailable until the async component has resolved. The example of OP would still fail, in my understanding.

So in summary:

  • We could improve this scenario a bit so that the exposed properties will be available once the component has resolved.
  • But this would still leave room for bugs and confusion as it they will be missing while it's still pending.

So fix or not, the general recommendation should be to call expose() before any top level await calls in setup.

@languanghao
Copy link

@LinusBorg Thank you for your reply.

Sometimes call expose() before any top level await calls will make code logic strange as I replied before. The exposed fuction invkoe var which defined in setup is very normal.

Consider exposed function need invoke a var which created by an async function, then we only have tow choises:

  1. define the var after the exposed function
const save = () => {
   // where is the declare code for recorder?
  console.log(recorder)
}
const recorder = await getRecorder()
  1. declare the var from const to let whithout init value before the exposed function
// delcare the recorder without init, so we need special the type, and we need change const to let
let recorder: RecorderType
const save = () => {
  console.log(recorder)
}
recorder = await getRecorder()

The first option violates the logic of writing code, we always invoke a var after we declared it. Here we change the code order, only because we need let the defineExpose work as expected.

The second option seems tobe a good choise than the first one, but still make the code reader confusion why we need split declare and init, where is the init code.

So I think we should find a way to improve the defineExposed macro to work better.

@jods4
Copy link
Contributor

jods4 commented Sep 27, 2024

@LinusBorg It would be nice if something was done to make defineExpose usable after await.
You can see this issue was referenced many times.
It's sometimes quite inconvenient to pass a strongly typed object to defineExpose when all its constituent won't be declared until later.

Your idea of having the compiler set a hasExpose flag on component sounds ok.
Or an alternative could be for the compiler to define instance.exposed to a temporary value in async component so that it exists and the ref is considered an exposed component, even if it will be re-defined later.

But that would still leave us with a challenge: While the async component is still pending, instance.exposeProxy would exist, but still be unaware of the sayHi function as expose() hasn't been called yet. That would mean that comp.sayHi would still be unavailable until the async component has resolved. The example of OP would still fail, in my understanding.

I don't think that's what happens.
The template ref will remain undefined until the async component resolves and is mounted in DOM. At this point, all the setup code has executed and the exposed object should be "complete".
You can observe that it seems to work in this Vue Playground.

Another argument is that this is actually what people have to do today.
The work-around is to pass an incomplete object to defineExpose and fill it in after await. We do that, as we have to.
If that incomplete exposed object was visible in template refs before async resolves, we would have these same issues, too.
Thankfully it seems to not be the case.

If I'm correct whether you can call defineExpose before or after await makes no difference in real-world usage, but it's a great improvement to DX, especially with TS typing. If it's just a matter of the compiler defining instance.exposed at the beginning of async components that contain defineExpose macro, I think Vue should do it.

@jods4
Copy link
Contributor

jods4 commented Sep 27, 2024

BTW, if Vue does not improve defineExpose to be usable after await, this limitation should be a warning in the docs of defineExpose.

@edison1105
Copy link
Member Author

@jods4
I've made a PR to fix this one. but I am not even sure it is the proper fix.
Please feel free to review.

@edison1105 edison1105 added 🐞 bug Something isn't working has workaround A workaround has been found to avoid the problem labels Sep 30, 2024
@jods4
Copy link
Contributor

jods4 commented Sep 30, 2024

@edison1105 To be honest it's not what I expected, based on @LinusBorg comment above.

My main worry with your queuePostRenderEffect approach is timing.
I'm afraid the exposed value would have a different timing than what's observable in sync components.
Your test waits until the child component is fully resolved, and Vue queue has been processed.
Devs usually expect that refs are set when onMounted is called, and I don't think it would be the case with your PR (to be fair: I have not run the code to verify).

The approach I assumed from Linus' description was this:

  1. The compiler is modified so that when an async component contains a defineExpose macro, then instance.exposed = {} is introduced at the beginning of setup() so that next step sees a defined exposed member.
    This will be later replaced by the right object when the user defineExpose is called (after awaiting).
  2. This code in getComponentPublicInstance is modified: https://github.com/vuejs/core/blob/main/packages/runtime-core/src/component.ts#L1179-L1194.
    Because of 1. it will see an exposed property and create the proxy. What remains to be done is being able to swap the instance when the component later calls defineExpose asynchronously.
    One cannot change the target of a Proxy, but I think there might be an opportunity to simplify code here.
    The proxy could wrap instance directly and perform similarly -- and support changing the exposed object.
    This might be a start?
instance.exposeProxy = new Proxy(instance, {
    get(target, key: string) {
      if (key in target.exposed) {
        return unref(target.exposed[key])
      } else if (key in publicPropertiesMap) {
        return publicPropertiesMap[key](instance)
      }
    },
    has(target, key: string) {
      return key in target.exposed || key in publicPropertiesMap
    },
  }))

It's worth noting that the proxy handler is identical for all instances, so it could be cached and reused for all components (saving a bit of memory).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has workaround A workaround has been found to avoid the problem 🐞 bug Something isn't working
Projects
None yet
4 participants