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

types(defineSlots): Support for generic keyof slots #8374

Merged
merged 5 commits into from
Dec 8, 2023

Conversation

pikax
Copy link
Member

@pikax pikax commented May 19, 2023

related vuejs/language-tools#3141
See comment: vuejs/language-tools#3141 (comment)

Currently defineSlots with keyof generic has the error of // Type '{}' has no call signatures.ts(2349), this PR fixes that error.

The issue is caused by Readonly, it seems it changes the type for calling the function. I think the type readonly safeness on typescript is less important than having generics working properly.

<script setup lang="ts" generic=" TCodes extends string ">
export interface Tab<TCode extends string> {
  code: TCode;
  label: string;
}
const props = defineProps<{
  links: Array<Tab<TCodes>>;
}>();

const slots = defineSlots<
  {
    [P in TCodes as `label:${P}`]: (props: { tab: any; label: string }) => void;
  } & {
    label?: (props: { tab: any; label: string }) => void;
  }
>();


// typescript error 
for (const col of props.links) {
  slots[`label:${col.code}`]?.({ col }); // Type '{}' has no call signatures.ts(2349)
}
</script>
<template>
  // error here
  <slot :name="`label:${tab.code!}`" :tab="tab" :label="tab.label">
    <slot name="label" :tab="tab" :label="tab.label">
      {{ tab.label }}
    </slot>
  </slot>
</template>

@jd-solanki
Copy link

Whoa! Thanks so much for quick action ❤️

@xiaoxiangmoe
Copy link
Contributor

@pikax Hi, I tried it and it seems not fix vuejs/language-tools#3141 now. Is there anything I miss?

@xiaoxiangmoe
Copy link
Contributor

xiaoxiangmoe commented May 19, 2023

In @jd-solanki 's demo. I update vue to 3.3.4 and Change StrictUnwrapSlotsType's Readonly<T> to T. And I found

<script lang="ts" setup generic="Row extends Record<string, unknown>">
import { objectKeys } from '../utils';
import { aTableProps, aTableSlots } from './meta';

const props = defineProps(aTableProps<Row>())

const _slots = aTableSlots<Row>(
    objectKeys(props.rows[0] || {}),
)
const slots = defineSlots<typeof _slots>()

for (const col of props.cols) {
    const slot = slots[`header-${col.name}`];

    slot?.({ col })
}
</script>

<template>
    <div class="wrapper">
        <div v-for="row in props.rows">
        <header v-for="(col, index) in props.cols" :key="index">
            <slot :name="`header-${col.name}`" :col="col"></slot>
            <!--  This expression is not callable. Type '{}' has no call signatures. -->           
        </header>
        </div>
    </div>
</template>

image

image

@pikax
Copy link
Member Author

pikax commented May 19, 2023

@xiaoxiangmoe please provide an contained example, otherwise there's too many running pieces that might get this broken.

I would discourage using props. on the template, there's a lot of things going on typescript side that might break the correct type.

Here's a full working self contained example:

<script lang="ts" setup generic="Row extends Record<string, unknown>">
const props = defineProps<{
  rows: Row[]
  cols: { name: keyof Row; label: string }[]
}>()

const slots = defineSlots<{
  [K in keyof Row as `header-${K & string}`]: (props: {
    col: { name: keyof Row; label: string }
  }) => any[]
}>()

for (const col of props.cols) {
  const slot = slots[`header-${String(col.name)}`]
  slot?.({ col })
}
</script>

<template>
  <div class="wrapper">
    <div v-for="row in rows">
      <header v-for="(col, index) in cols" :key="index">
        <slot :name="`header-${String(col.name)}`" :col="col"></slot>
        <!--  This expression is not callable. Type '{}' has no call signatures. -->
      </header>
    </div>
  </div>
</template>
image

@jd-solanki
Copy link

@pikax I want to point out that we also have additional slot that is concated using & like in your PR's test but that isn't present in your self contained example.

Additionally, I want to point out that user might separate component's metas like props, slots, emits in another ts file and import it in SFC like I do in my UI lib anu. Thing to consider it when we import object from another file we need to use as const sometimes to make it work as expected. e.g. vue-component-meta's docs: https://github.com/vuejs/language-tools/tree/master/packages/vue-component-meta#required (notice as const)

@pikax
Copy link
Member Author

pikax commented May 19, 2023

@pikax I want to point out that we also have additional slot that is concated using & like in your PR's test but that isn't present in your self contained example.

Because that's very straightforward and the adding extra doesn't reproduce the bug.
image

Additionally, I want to point out that user might separate component's metas like props, slots, emits in another ts file and import it in SFC like I do in my UI lib anu. Thing to consider it when we import object from another file we need to use as const sometimes to make it work as expected. e.g. vue-component-meta's docs: https://github.com/vuejs/language-tools/tree/master/packages/vue-component-meta#required (notice as const)

You're free to import from other files, but using defineSlots on a way that's not correct, that's the issue. Your aTableSlots is hacky, it mixis unnecessarily runtime with typescript, the runtime behaviour doesn't mean much (AFAIK), most likely there's an issue with your function if the defineSlots is working as intended when using typescript only syntax.

Just to be clear Type '{}' has no call signatures.ts(2349) error is only typescript.

@localusercamp
Copy link

Any news on merging this?
I really miss this feature =)

@github-actions
Copy link

github-actions bot commented Oct 6, 2023

Size Report

Bundles

File Size Gzip Brotli
runtime-dom.global.prod.js 87 kB 33.1 kB 29.9 kB
vue.global.prod.js 133 kB 49.9 kB 44.8 kB

Usages

Name Size Gzip Brotli
createApp 48.3 kB 19 kB 17.4 kB
createSSRApp 51.5 kB 20.3 kB 18.5 kB
defineCustomElement 50.7 kB 19.8 kB 18 kB
overall 61.7 kB 23.9 kB 21.7 kB

Copy link

codspeed-hq bot commented Dec 8, 2023

CodSpeed Performance Report

Merging #8374 will not alter performance

Comparing types_support_generic_slots_based_on_keys (1f4b0b2) with main (0d61b42)

Summary

✅ 53 untouched benchmarks

@yyx990803 yyx990803 merged commit 213eba4 into main Dec 8, 2023
18 checks passed
@yyx990803 yyx990803 deleted the types_support_generic_slots_based_on_keys branch December 8, 2023 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants