vuejs / core

🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
https://vuejs.org/
MIT License
46.79k stars 8.21k forks source link

Compiler should warn about duplicate/conditional slots #5700

Open jaa134 opened 2 years ago

jaa134 commented 2 years ago

Version

3.2.31

Reproduction link

vue-next-template-explorer.netlify.app

Steps to reproduce

Look at the AST for the SFC. In the dev console, you can see the component has only one child. image

The workaround example (from below) has the part that is missing. branches is present for conditional rendering image

What is expected?

Both named-slots are registered in the AST.

What is actually happening?

Only one of the conditional templates with named slots is registered in the AST. The second is not.


Reference: https://github.com/johnsoncodehk/volar/issues/1150

Workaround: not put a condition on template of named slot but instead make the content conditional

SFC that generates bad AST

<script
  setup
  lang="ts"
>
  import { ref } from 'vue';
  import ComponentWithSlots from '@/src/foobar';

  const aVariableThatExists = ref(true);
</script>

<template>
  <ComponentWithSlots>
    <template
      #content
      v-if="aVariableThatExists"
    >
      <div>{{ aVariableThatExists }}</div>
    </template>
    <template
      #content
      v-else
    >
      <div>{{ thisVariableDoesNotExist }}</div>
      <div>{{ thisShouldBeUnderlinedInRed }}</div>
    </template>
  </ComponentWithSlots>
</template>

Workaround SFC

Move the conditional logic into the children of a single slotted template. Define an extra set of template tags that the conditional logic can live on:

<script
  setup
  lang="ts"
>
  import { ref } from 'vue';
  import ComponentWithSlots from '@/src/foobar';

  const aVariableThatExists = ref(true);
</script>

<template>
  <ComponentWithSlots>
    <template
      #content
    >
      <template v-if="aVariableThatExists">
        <div>{{ aVariableThatExists }}</div>
      </template>
      <template v-else>
        <div>{{ thisVariableDoesNotExist }}</div>
      </template>
    </template>
  </ComponentWithSlots>
</template>
jaa134 commented 2 years ago

This issue was originally created as a Volar issue at which point we determined it is a legitimate Vue bug.

LinusBorg commented 2 years ago

I think we had an issue about that before and the consensus was that v-if on slot templates isn't allowed, and the "workaround" shown here is the intended way to use conditional slots,

Provided my memory is correct, we likely should document this better, and have the compiler throw an error in this case.

jaa134 commented 2 years ago

A compiler error seems like a good decision to keep developers from making the same mistake!

AlexandreBonaventure commented 11 months ago

I think we had an issue about that before and the consensus was that v-if on slot templates isn't allowed, and the "workaround" shown here is the intended way to use conditional slots

Sorry to ping @LinusBorg, but I feel like a lot of vue users are using this pattern extensively. If I follow you correctly, you are saying that this is not supported anymore ? It feels like a big breaking change, see https://github.com/vuejs/core/issues/6651 for instance.

Rendering slots conditionally is the only way to work reliably with existence check provided by $slots.xxx

Capture d’écran, le 2023-10-02 à 11 22 00

The workaround does not help with this.

Just wondering if this topic should be raised again and bumped up, since it seems like a legit issue.

In our codebase, Vue language tool is not functioning properly and eslint autofixed is removing some imports because it doesn't know about all AST Nodes :(