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.

Sign up for GitHub

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

Jump to bottom

props on template are not checked #4539

Open
brc-dd opened this issue Jul 6, 2024 · 9 comments · May be fixed by #4544
Open

props on template are not checked #4539

brc-dd opened this issue Jul 6, 2024 · 9 comments · May be fixed by #4544
Labels
bug Something isn't working good reproduction ✨ This issue provides a good reproduction, we will be able to investigate it first

Comments

@brc-dd
Copy link
Member

brc-dd commented Jul 6, 2024 •
edited
Loading

Vue - Official extension or vue-tsc version

2.0.26

VSCode version

1.92.0-insider

Vue version

3.4.31

TypeScript version

5.5.3

System Info

No response

Steps to reproduce

  1. git clone [email protected]:brc-dd/volar-template-key-bug.git
  2. pnpm i
  3. pnpm vue-tsc --noEmit

Pasting here for completeness:

<script setup lang="ts"></script>

<template>
  <template v-if="true" :key=""></template>
  <template v-if="true" :key="{}"></template>

  <div v-if="true" :key=""></div>
  <div v-if="true" :key="{}"></div>
</template>

What is expected?

Error on line 4, 5, 7, 8 of Foo.vue

What is actually happening?

Error is only on line 7, 8.

There should be an error on line 4 saying v-bind is missing expression.

There should be an error on line 5 saying Type '{}' is not assignable to type 'PropertyKey | undefined'.

Link to minimal reproduction

https://github.com/brc-dd/volar-template-key-bug

Any additional comments?

Is it possible to show vue errors when running vue-tsc? They are currently shown only in the editor. (Line 7 in the above example.)
@brc-dd brc-dd changed the title key is not type-checked on template props on template are not checked Jul 6, 2024
@davidmatter davidmatter added good reproduction ✨ This issue provides a good reproduction, we will be able to investigate it first bug Something isn't working and removed pending triage labels Jul 6, 2024
@davidmatter
Copy link
Collaborator

davidmatter commented Jul 7, 2024 •
edited
Loading

Input

<script setup lang="ts"></script>

<template>
  <template v-if="true" :key="">
	<div></div>
  </template>
  <template v-if="true" :key="{}">
	<div></div>
  </template>

  <div v-if="true" :key=""></div>
  <div v-if="true" :key="{}"></div>
</template>

Output

// ...
if (true) {
__VLS_elementAsFunction(__VLS_intrinsicElements.div, __VLS_intrinsicElements.div)({});
}
if (true) {
__VLS_elementAsFunction(__VLS_intrinsicElements.div, __VLS_intrinsicElements.div)({});
}
if (true) {
__VLS_elementAsFunction(__VLS_intrinsicElements.div, __VLS_intrinsicElements.div)({key: ((__VLS_ctx.)), });
// @ts-ignore
[,];
}
if (true) {
__VLS_elementAsFunction(__VLS_intrinsicElements.div, __VLS_intrinsicElements.div)({key: (({})), });
}
// ...

Should?

// ...
if (true) {
- __VLS_elementAsFunction(__VLS_intrinsicElements.div, __VLS_intrinsicElements.div)({});
+ __VLS_elementAsFunction(__VLS_intrinsicElements.div, __VLS_intrinsicElements.div)({key: ((undefined))});
}
if (true) {
- __VLS_elementAsFunction(__VLS_intrinsicElements.div, __VLS_intrinsicElements.div)({});
+ __VLS_elementAsFunction(__VLS_intrinsicElements.div, __VLS_intrinsicElements.div)({key: (({}))});
}
if (true) {
- __VLS_elementAsFunction(__VLS_intrinsicElements.div, __VLS_intrinsicElements.div)({key: ((__VLS_ctx.)), });
+ __VLS_elementAsFunction(__VLS_intrinsicElements.div, __VLS_intrinsicElements.div)({key: ((undefined))});
// @ts-ignore
[,];
}
if (true) {
__VLS_elementAsFunction(__VLS_intrinsicElements.div, __VLS_intrinsicElements.div)({key: (({})), });
}
// ...

@so1ve
Copy link
Member

so1ve commented Jul 8, 2024

https://github.com/vuejs/core/blob/93324b2ec06b26662b77abc2a75d21ecbe8913db/packages/compiler-core/src/transforms/vIf.ts#L206-L216

A <template> with v-if will be turned into an IF_BRANCH node, we should handle it in https://github.com/vuejs/language-tools/blob/master/packages/language-core/lib/codegen/template/elementDirectives.ts.

@so1ve so1ve linked a pull request Jul 8, 2024 that will close this issue
Draft
@brc-dd
Copy link
Member Author

brc-dd commented Jul 8, 2024 •
edited
Loading

Ah, it's not working for v-for in template's either. I had put v-if for easier repro. v-for + key is more common usage.

@so1ve
Copy link
Member

so1ve commented Jul 8, 2024

Should be fixed by cb54032? Do you mean there is no type checking?

@brc-dd
Copy link
Member Author

brc-dd commented Jul 8, 2024 •
edited
Loading

Do you mean there is no type checking?

Yeah it's not there for key. This should've shown error in line 3:

<script setup lang="ts"></script>

<template>
  <template v-for="x in [1]" :key="{}">
    <div>{{ x }}</div>
  </template>
</template>

@so1ve
Copy link
Member

so1ve commented Jul 8, 2024

Got it 👐

@so1ve
Copy link
Member

so1ve commented Jul 8, 2024

My PR doesn't report v-bind is missing expression, since CompilerDOM.compile() itself doesn't throw on those invalid directives.

@brc-dd
Copy link
Member Author

brc-dd commented Jul 8, 2024

Ah yeah. It can be left I guess. I'll track it on the core repo. Looks like compiler-dom is generating invalid JS for <template v-for="x in [1]" :key="" /> without complaining.

@so1ve
Copy link
Member

so1ve commented Jul 8, 2024

Yeah, v-if with :key generates { key: 0 } while v-for with :key generates { key: }

@so1ve so1ve mentioned this issue Jul 9, 2024
Open
@KazariEX KazariEX mentioned this issue Oct 27, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working good reproduction ✨ This issue provides a good reproduction, we will be able to investigate it first
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(language-core): generate codes for :key prop on v-for and v-if node so1ve/language-tools
3 participants
@davidmatter @brc-dd @so1ve