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

feat: allow String as type of key #6448

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Shinigami92
Copy link
Contributor

@Shinigami92 Shinigami92 commented Aug 11, 2022

This is a TypeScript only improvement
It allows to pass a special case like this:

<script setup lang="ts">
// import type { MyItem } from '@/shared/models/MyItem';
import { ref } from 'vue';

interface NodeId extends String {
//                       ^^^^^^ this is the important part for the reason of this PR
  _nodeIdBrand: string;
}

interface MyItemId extends NodeId {
  _myItemIdBrand: string;
}

interface MyItem extends BaseNode<MyItemId> {
  label: string;
}

const items = ref<MyItem[]>([/*...*/]);

function toAny(value: any): any {
  return value;
}
</script>

<template lang="pug">
ul
  //- Previously I needed this: li(v-for="item in items", :key="toAny(item.id)")
  li(v-for="item in items", :key="item.id")
    span {{ item.label }}
</template>

@netlify
Copy link

netlify bot commented Aug 11, 2022

Deploy Preview for vue-sfc-playground failed.

Name Link
🔨 Latest commit a981097
🔍 Latest deploy log https://app.netlify.com/sites/vue-sfc-playground/deploys/62f4ad0ccb492b0009e5d577

@netlify
Copy link

netlify bot commented Aug 11, 2022

Deploy Preview for vuejs-coverage failed.

Name Link
🔨 Latest commit a981097
🔍 Latest deploy log https://app.netlify.com/sites/vuejs-coverage/deploys/62f4ad0c56b0b700099015cb

@Shinigami92
Copy link
Contributor Author

I'm unsure if the failing e2e is related to my changes 🤔
Could we try to rerun it?

@Shinigami92 Shinigami92 marked this pull request as ready for review August 11, 2022 08:28
@KaelWD
Copy link
Contributor

KaelWD commented Aug 15, 2022

#2633 - keys can basically be {}

@Shinigami92
Copy link
Contributor Author

#2633 - keys can basically be {}

I saw that issue, but hoped that my can be merged earlier on the road to that

@KaelWD
Copy link
Contributor

KaelWD commented Aug 15, 2022

It already works at runtime, the types just need to be changed. {} would work for any object instead of having a special case just for String.

@Shinigami92
Copy link
Contributor Author

Shinigami92 commented Aug 16, 2022

Yeah I know, literally the first sentence in the PR description says that 😅
There is already another PR to solve the issue you are trying to explain
But this is not my issue and it seems like it doesn't get priority
So I opened this one to give at least a fix for my case
I do not use objects and I don't want to use objects as key as I think that is bad practice

@edison1105 edison1105 added scope: types 🧹 p1-chore Priority 1: this doesn't change code behavior. ready for review This PR requires more reviews labels Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧹 p1-chore Priority 1: this doesn't change code behavior. ready for review This PR requires more reviews scope: types
Projects
Development

Successfully merging this pull request may close these issues.

3 participants