Strict excessive property type-checking #460
Description
I just realized the client will accept basically any variables in query-calls.
By which I mean, something like this is apparently valid:
await client.mutate({
mutation: UPDATE_DOCUMENT,
variables: {
id,
filename,
createdAt,
propertyThatTotallyDoesntExist: 'whatever', // valid
},
})Apparently, this is due to some messed-up type-checking issue they won't/can't fix.
Here's an example demonstrating the issue:
interface Foo {
one: string
two: string
three: string
}
function hello<T extends Foo>(foo: T) { }
hello({
one: 'one',
two: 'two',
three: 'three',
lol: 'wtf' // ❌
})
hello({
one: 'one',
two: 'two',
// three: 'three',
lol: 'wtf' // ✅
})That is, as long as the variables that must be there are there, it will accept anything.
Or in other words, type-checking for excess properties is for some reason conditional on satisfying any required properties.
What this means is your codebase could be full of dead code and you would never know it.
To make matters worse, because the type is generic, any extra properties will appear in an IDE to be real properties:
Not only is it not type-checking, it's also misleading you to think these properties have types and could be in use. 😢
There is a way to make this type-checking stricter:
interface Foo {
one: string
two: string
three: string
}
function hello<T extends Foo & Record<string, never>>(foo: T) { }
hello({
one: 'one',
two: 'two',
three: 'three',
lol: 'wtf' // ✅
})
hello({
one: 'one',
two: 'two',
// three: 'three',
lol: 'wtf' // ✅
})If applied to the query methods in the client, this would obviously be a huge breaking change, however.
Will it break anything that shouldn't be broken though? I'm still kind of new to GraphQL, so I can't say.
I would love to see this addressed though, as I suspect our codebase may be full of dead code that just can't be checked.
What do you think?