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

Type checks by typescript fail on Vue.set/delete #8707

Closed
nel215 opened this issue Aug 25, 2018 · 9 comments
Closed

Type checks by typescript fail on Vue.set/delete #8707

nel215 opened this issue Aug 25, 2018 · 9 comments

Comments

@nel215
Copy link
Contributor

nel215 commented Aug 25, 2018

Version

2.5.17

Reproduction link

https://github.com/nel215/vue-sandbox

Steps to reproduce

$ git clone https://github.com/nel215/vue-sandbox.git
$ cd vue-sandbox
$ yarn
$ ./node_modules/.bin/tsc index.ts
index.ts:5:9 - error TS2345: Argument of type '{}' is not assignable to parameter of type '"a"[]'.
  Property 'length' is missing in type '{}'.

5 Vue.set(data.obj, 1, 'a')
          ~~~~~~~~

index.ts:6:12 - error TS2345: Argument of type '{}' is not assignable to parameter of type '{}[]'.
  Property 'length' is missing in type '{}'.

6 Vue.delete(data.obj, 1)

What is expected?

Type checks by typescript succeed on this case.

What is actually happening?

Type checks by typescript fail on this case.


Can we use number as key of object on Vue.set/delete? If it's yes, I will create a PR which fixes types/vue.d.ts.

@ktsn
Copy link
Member

ktsn commented Aug 25, 2018

Shouldn't it be Vue.set(data.obj, '1', 'a') if data.obj is an object?

@nel215
Copy link
Contributor Author

nel215 commented Aug 25, 2018

Using number as key of Object is not recommended? In my actual case of Vue.set(data.objects, obj.id, obj), obj.id is identifier and the type is number. So, I'd appreciate if we can use number with Object.

@ktsn
Copy link
Member

ktsn commented Aug 25, 2018

I'm not sure that allowing such implicit type conversion in typings. I think it's enough to convert the id to string by using String constructor or toString.

@posva
Copy link
Member

posva commented Aug 25, 2018

In js, the conversion is implicit but if you want to use strict typing, then you should cast your number into a string. Doing a[1] or a['1']` in js will implicitly access the same thing. That being said, typescript allows it without warning, so I think we should support it too

const a: Record<string, boolean> = {}
a['1'] = false
a[2] = true

@KaelWD
Copy link
Contributor

KaelWD commented Aug 25, 2018

Note that the inverse is not true for arrays:

const a: Record<string, boolean> = {}
a['1'] = false
a[2] = true

const b: boolean[] = []
b['1'] = false  // Element implicitly has an 'any' type because index expression is not of type 'number'
b[2] = true

Could also go with fully strict instead, but that might annoy a few people

declare function set<T extends O[K], K extends keyof O, O>(object: O, key: K, value: T): T

const c: Record<string, boolean> = {}
const d: boolean[] = []
const e: { foo: 'bar' | 'baz' } = { foo: 'bar' }

set(c, '1', false)
set(c, 2, false)          // nope
set(c, '3', 'a string')   // nope

set(d, '1', false)        // nope
set(d, 2, false)
set(d, 3, 'a string')     // nope

set(e, '1', false)        // nope
set(e, 2, false)          // nope
set(e, 'foo', 'a string') // nope
set(e, 'bar', 'baz')      // nope
set(e, 'foo', 'baz')

@nel215

This comment has been minimized.

@posva

This comment has been minimized.

@nel215

This comment has been minimized.

@yyx990803
Copy link
Member

Closed via #8709

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants