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

Vue2.7 defineEmits/defineProps compile error #13122

Closed
DeAntiWang opened this issue Nov 23, 2023 · 6 comments
Closed

Vue2.7 defineEmits/defineProps compile error #13122

DeAntiWang opened this issue Nov 23, 2023 · 6 comments

Comments

@DeAntiWang
Copy link

Version

2.7.15

Reproduction link

codepen.io

Steps to reproduce

compile the code below:

<template>
  <div></div>
</template>

<script setup>
  const emits = (Date.now(), defineEmits(['input']))
</script>

defineEmits will not transform to options member emits of defineComponent in compiled dist.
same on the defineProps

What is expected?

the CallExpression defineEmits be compiled normally to a option member emits of defineComponent in compiled dist.

What is actually happening?

the CallExpression defineEmits is not be compiled.


it should a bug of @vue/compile-sfc.
the calling of isCallOf at processDefineEmits and processDefineProps is only check CallExpression but not check SequenceExpression.

@posva
Copy link
Member

posva commented Nov 23, 2023

How did you find this problem? I don’t see why one would write a sequence when defining the emit function?

@DeAntiWang
Copy link
Author

How did you find this problem? I don’t see why one would write a sequence when defining the emit function?

Because of istanbuljs's instrumentation process. The VariableDeclaration code after instrumenting will be compiled to SequenceExpression.

Like this code:

<script setup>
const emits = defineEmits(['input']);
</script>

after intrument, it will become like this:

<script setup>
// some unrelated report init code
const emits = (report(0), defineEmits(['input']));
</script>

@posva
Copy link
Member

posva commented Nov 23, 2023

Aah I see! Given that Vitest manages to pull off the coverage on Vue components, this might be a misconfiguration: Istanbul should maybe instrument the code after and use source maps to refer to the original place

@DeAntiWang
Copy link
Author

Aah I see! Given that Vitest manages to pull off the coverage on Vue components, this might be a misconfiguration: Istanbul should maybe instrument the code after and use source maps to refer to the original place

oh cool! thanks, I'll have a look.
but there are many complex reasons leading us to choose the way to instrument the code first.

Supporting SuequenceExpression for @vue/compile-sfc seems like a nice thing too. Maybe I'll try to make a PullRequest for that.

@DeAntiWang
Copy link
Author

have submitted a PullRequest(#13124) about this

@yyx990803
Copy link
Member

I'm not convinced that this complexity should be dealt with in Vue - instrumenting raw SFC code just sounds wrong. Only handling sequences also seems like an incomplete fix, as there are other cases where the macros won't be compiled. For example, what if the instrumentation wraps the defineXXX macros in conditional branches?

Even just for sequences, the PR also has even more edge cases:

  • It assumes the defineXXX always is the last in the sequence, so it would fail for

    const props = (Date.now(), defineProps(['input']), Date.now())
  • In the multi-declaration case, it also doesn't account for duplicated placeholders:

    const foo = 1, props = (Date.now(), defineProps(['input']))
    const bar = 1, emits = (Date.now(), defineEmits(['input']))

    Generated code will error because two variables named _ are declared.

In principle, compileScript expects to work with the exact code written by the developer, so we do not account for extremely unlikely usage like sequence expressions. But if the goal is to make the logic robust enough that it can handle unknown instrumentation by other tools, then the complexity will explode and I don't think Vue should be responsible for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants