vuejs / vue

This is the repo for Vue 2. For Vue 3, go to https://github.com/vuejs/core
http://v2.vuejs.org
MIT License
207.34k stars 33.63k forks source link

Inconsistent Vue.set() and Vue.delete() TypeScript types #9355

Open zlumer opened 5 years ago

zlumer commented 5 years ago

Version

2.5.22

Reproduction link

https://bit.ly/2W6bvIu

Steps to reproduce

  1. Make sure you have strict: true in your tsconfig.json to handle all type mismatch cases.
  2. Run TypeScript compiler against provided code.

What is expected?

Vue mimics TypeScript behaviour, triggers compiler errors on not allowed operations.

What is actually happening?

Vue doesn't trigger any compilation errors.


As discussed in #9347 and #9353, TypeScript typings for Vue.set() and Vue.delete() are not consistent with TypeScript behavior. While TypeScript helps catch bugs at compile time with its type system, Vue breaks certain TypeScript rules due to incomplete type definitions in vue.d.ts

I propose changing Vue.set()/Vue.delete() typings to the following:

set<T, K extends keyof T>(object: T, key: K, value: T[K]): T[K];
delete<T>(object: T, key: keyof T): void;

This will match TypeScript behaviour on provided examples.

posva commented 5 years ago

This would be a breaking change set allows to dynamically define properties that do not exist as well as properties coming from arbitrary sources like user input:

const obj = {}
// dynamicName could come from end user
set(obj, dynamicName, value)

If we add those typings we would be breaking this usage. Same goes for delete, it can delete dynamic properties. We would need to force users to type those objects as Record<string, any>. I think this makes sense though, so maybe we should ensure this for Vue 3 instead. cc @ktsn, @HerringtonDarkholme

zlumer commented 5 years ago

I agree that this is a breaking change not suitable for patch version (not sure about minor/major though).

posva commented 5 years ago

breaking changes are only possible in major versions (Vue 3)