Closed jcalz closed 2 years ago
related discussion also in https://github.com/Microsoft/TypeScript/issues/10715 about type guards "evolving" the type of an expression.
This! Currently, we have the odd situation that when you write:
if ("assign" in Object) {
Object.assign(target, ...args);
}
When the ES6 typings aren't loaded, it will narrow Object
down to never
in the if
statement!
@GregRos please see https://github.com/Microsoft/TypeScript/issues/21517
@mhegazy Yup, I know. I was giving it as an example for what this suggestion would fix. I know why it's happening.
Let's add some other ways of asserting property existence from #25720 to this proposal:
let x: unknown;
// All these should narrow x to {prop: unknown} (s/any/unknown/ from original proposal by Matt)
"prop" in x;
x.prop != null;
x.prop !== undefined;
typeof x.prop !== "undefined";
// typeof should work on properties of the unknown variable
typeof x.prop === "string"; // should narrow x to {prop: string}
@mattmccutchen I think you'd want those to narrow to {prop: unknown}
rather than {prop: any}
wouldn't you?
@mattmccutchen The problem is that trying to access a property on null/undefined will throw an error at runtime. I would support adding those type guards, but only on the object
type. (If there were a way to get an unknownWithoutNullOrUndefined
type, that would be even better)
@jcalz can you highlight the differences between #10485 and what you'd want to happen?
TL;DR: #10485 narrows by filtering union constituents. This suggestion narrows by adding properties to the type.
In cases where y
is not a union type or none of the constituents have an explicit property named x
, the test if (x in y)
should not try to filter union constituents, but should instead narrow y
by intersecting its type with Record<typeof x, unknown>
.
In #10485, the type of the object you're checking with in
is meant to be a union, and the type guard filters that union based on whether the constituents do or do not explicitly have a property of the relevant name:
type A = { w: string, x: number };
type B = { y: string, z: number };
function foo(p: A | B): number {
if ('w' in p) {
return p.x; // p is narrowed to A
} else {
return p.z; // p is narrowed to B
}
}
Note that this isn't exactly sound, since you can call
foo({ y: "oops", z: 100, w: true });
but the point of property checking on unions is usually to use that property as a discriminant, and the sound behavior would probably annoy the heck out of developers.
Compare to the following:
function bar(p: {}): string {
if ('w' in p) {
return String(p.w); // error?! ☹
} else {
return "nope";
}
}
It is surprising that after what feels like an explicit test for p.w
, the compiler still doesn't know that p.w
exists. Worse, p
is narrowed to never
, probably because of #10485.
What I want to see happen is:
x in y
check be equivalent to the type guard y is typeof y & Record<typeof x, unknown>
(modulo the bug in #18538).@jcalz This guard doesn't work with partial interfaces
function foo(x: {foo: "bar"} | {toFixed?: () => any}) {
if (inOperator("toFixed", x)) {
x.toFixed(); // Error, Object is of type unknown
}
}
@sirian Yeah, strictly speaking, all you know is that x.toFixed
exists, since a value of type {foo: "bar"}
may contain a toFixed
property (e.g.,
foo(Object.assign({ foo: "bar" as "bar" }, { toFixed: "whoops" })); // no error
), but assuming people still want the current behavior in #10485 where we eliminate {foo: "bar"}
from consideration as soon as we find a toFixed
property, then this suggestion is to apply the inOperator()
behavior after that elimination.
So in your case, x
would first be narrowed to {toFixed?: () => any}
as per #10485 and then to {toFixed: (() => any) | undefined}
via something like inOperator()
... meaning that toFixed
is definitely there but it might be undefined
(since ?
is ambiguous about whether the property is actually missing or present but undefined.)
If you want a better idea what the behavior would end up being like, try the following complex user-defined type guard using conditional types:
type Discriminate<U, K> = ( U extends any ? K extends keyof Required<U> ? U : never : never )
extends infer D ? [D] extends [never] ? U : D : never
function hybridInOperator<K extends keyof any, T>(
k: K,
o: T
): o is Discriminate<T, K> & Record<K, unknown> {
return k in o;
}
producing:
function foo(x: { foo: "bar" } | { toFixed?: () => any }) {
if (hybridInOperator("toFixed", x)) {
x.toFixed(); // error, possibly undefined
if (x.toFixed) x.toFixed(); // okay
}
}
Does that seem better?
@jcalz Better, but there is another problem with type infer. Look at example
function foo(x: { foo: "bar" } | Number | { toFixed?: () => any }) {
if (hybridInOperator("toFixed", x)) {
x.toFixed(); // no error. since x resolved as Number
if (x.toFixed) x.toFixed(); // okay
}
}
I also tried various type guards. But always found a new counterexample(
Upd. As a quick fix - if you change o is Discriminate<T, K> & Record<K, unknown>
to o is Extract<T, Discriminate<T, K>>
. then x will be resolved as Number | { toFixed?: () => any }
Upd2.
So in your case, x would first be narrowed to {toFixed?: () => any} as per #10485 and then to {toFixed: (() => any) | undefined}
Not right... toFixed
would be narrowed to unknown
... Look at example. So and error in https://github.com/Microsoft/TypeScript/issues/21732#issuecomment-414126867 screenshot was not "object is possibly undefined"
I don't know how worthwhile it is to come up with a user-defined type guard which exactly mimics the desired behavior of in
narrowing, in light of the variety of edge cases. In this case, something weird is happening, since the hybridOperator()
returns something like x is Number | { toFixed: undefined | (() => any) };
which is wider than Number
. The fact that x
narrows to Number
inside the then-clause looks like some kind of compiler bug (edit: as per #26551, it is not considered a bug, but it is inconsistent and makes it more difficult to design a perfect type guard). I'd rather not get sidetracked here with that.
The "quick fix" has toFixed
as optional, but it should be required... which is why I was intersecting with {toFixed: unknown}
in the first place.
re: Upd2, I think you're missing that the suggestion is to apply the inOperator() behavior after the elimination that happens in #10485 ? Not sure if I wasn't clear about that.
@jcalz Maybe add smth like & Pick<Required<T>, K & keyof <Required<T>>>
? I'll try to experiment at home )
@jcalz what about this one?
type Discriminate<U, K extends PropertyKey> =
U extends any
? K extends keyof U ? U : U & Record<K, unknown>
: never;
function inOperator<K extends PropertyKey, T>(k: K, o: T): o is Discriminate<T, K> {
return k in o;
}
function foo(x: null | string | number | Number | { toFixed?: () => any } | { foo: 1 }) {
if (inOperator("toFixed", x)) {
// x is number | Number | { toFixed?: () => any | undefined }
x.toFixed && x.toFixed();
}
}
function g<K extends string, T>(key: K, genericObj: T, concreteObj: {foo: string}) {
if (inOperator('a', concreteObj)) {
concreteObj.a // unknown
}
if (inOperator('a', genericObj)) {
genericObj.a // any
}
if (inOperator(key, concreteObj)) {
concreteObj[key]; // unknown
}
if (inOperator(key, genericObj)) {
genericObj[key] // any
}
}
Looks good at first glance!
@jcalz There is a dilemma. how should work typeguard?
function foo(x: {foo?: number} | {bar?: string}) {
if (inOperator("foo", x)) {
// x is {foo: number | undefined}
// or
// x is {foo: number | undefined} | {foo: unknown, bar?: string}
}
}
or in this case:
function foo(x: {foo?: number} | string) {
if (inOperator("foo", x)) {
// x is {foo: number | undefined}
// or
// x is {foo: number | undefined} | string & {foo: unknown}
}
}
It should be the first one in both cases, {foo: number | undefined}
, as implied by the bulleted list at the bottom of this comment.
@jcalz So if we extract types rather than widen it with additional property - what should be at this examples?
function foo(x: {bar?: number}) {
if (inOperator("foo", x)) {
// x is never? so is x.foo not accessible?
}
}
function foo(x: object) {
if (inOperator("foo", x)) {
// x is never? so is x.foo not accessible?
}
}
It should be {bar?: number, foo: unknown}
and {foo: unknown}
respectively, as implied by the same bulleted list. I can't tell if that isn't clear or if you're not reading it. 🤕
- Only do the narrowing in #10485 if the type is a union where at least one constituent explicitly features the relevant property (edit: optional properties count as "explicit" also).
- Otherwise (or afterwards), have the
x in y
check be equivalent to the type guardy is typeof y & Record<typeof x, unknown>
(modulo the bug in #18538).
Looks like double standards. Сorrect me if I'm wrong
const x1: {foo?: number} | {bar:? number}
const x2: {bar?: number}
if (inOperator("foo", x1)) {
// x1 is {foo: number | undefined}
// x1.bar is not accessible
} else {
// x1 is {bar?: number}
}
if (inOperator("foo", x2)) {
// x1 is {bar?: number, foo: unknown}
} else {
// x1 is never
// x1.bar is not accessible
}
So that's why i say dilemma
) Sometimes this behavior widening types with additional props, but sometimes it's extracting types
That’s the sad truth. Extracting types is unsound, but desirable because it makes all unions act like discriminated unions, which is often what people are trying to do when checking for the existence of a property.
Asserting existence of a property is sound, sometimes useful, and always better than nothing... but I can’t realistically suggest doing that instead of extracting types.
Hence the hybrid approach I’ve laid out (multiple times) here. Hope that clears things up.
Agree with you. Just ask to know is this expected and desired behaviour. Forgot to remove optional flag in https://github.com/Microsoft/TypeScript/issues/21732#issuecomment-414499396
Updated version below with some tests. Check pls @jcalz
type Require<T, K extends keyof T> = T & { [P in K]-?: T[P] }
type Ensure<U, K extends PropertyKey> =
K extends keyof U
? Require<U, K>
: U & Record<K, unknown>
declare function has<T, K extends PropertyKey>(o: T, k: K): o is Ensure<T, K>;
type FakeNumber = { toFixed?: (fractionDigits?: number) => string };
type Test = null | number | FakeNumber;
declare const x: Ensure<Test, "toFixed">;
// Check as type
x.toFixed();
x.toPrecision(); // error - ok
const n: Number = x; // error - ok
// Check as type guard
if (has(x, "toFixed")) {
x.toFixed();
x.toPrecision(); // error - ok
const n: Number = x; // error - ok
}
if (has(x, "toPrecision")) {
x.toFixed();
x.toPrecision();
const n: Number = x;
}
function g<K extends string, T>(key: K, t: T, foo: { foo: string }, o: object) {
// check foo: {foo: string}
if (has(foo, 'a')) {
foo.a
foo.b // error - ok
foo[key] // error - ok
}
if (has(foo, key)) {
foo[key];
foo.a // no error. expected?
}
// check t: T
if (has(t, 'a')) {
t.a
t.b // error - ok
t[key] // error - ok
}
if (has(t, key)) {
t[key]
t.a // no error. expected?
}
// check o: object
if (has(o, "a")) {
o.a
o.b // error - ok
o[key] // error - ok
}
if (has(o, key)) {
o[key];
o.a // no error. expected?
}
}
@jcalz Ohh... type guard works bit different. it make additional type extraction - so previous answer is still wrong.
I mean
Ensure<number | FakeNumber, "toFixed">
is number | Require<FakeNumber, "toFixed">
But when used as type guard result will be Extract<T, Ensure<T, K>>>
. so:
type FakeNumber = { toFixed?: (fractionDigits?: number) => string };
declare const x: null | number | FakeNumber;
if (has(x, "toFixed")) {
// x is number
} else {
// x is null | FakeNumber
}
Just wanted to add, given @mattmccutchen's comment from July 17, that use of the in
operator on a value of type unknown
probably shouldn't be allowed without first using type narrowing using something like typeof value === 'object'
(#26327).
> 'foo' in 1
TypeError: Cannot use 'in' operator to search for 'foo' in 1
So:
function foo (value: unknown) {
console.log('bar' in value) // Object is of type 'unknown'. -- like now
if (typeof value === 'object') {
console.log('baz' in value) // No error
}
}
I would like this as well to implement validation code without unsafe type casts.
For example :
class ErrorFrame {
constructor(
public id: string,
public code: string,
public type: string,
public message: string
) { }
}
class Parser {
validateBody ( body: Buffer): { error: string, obj: unknown } {
let jsonBody
try {
jsonBody = JSON.parse(body.toString())
} catch (err) {
return { error: 'expected body to be json', obj: null }
}
return { error: '', obj: jsonBody }
}
parse (body: Buffer): { error: string, value?: ErrorFrame } {
const { error, obj } = this.validateBody(body)
if (error) {
return { error: error }
}
if (typeof obj !== 'object' || obj === null) {
return { error: 'expected json body to be object' }
}
if (!('id' in obj) || typeof obj.id !== 'string') {
return { error: 'expected body to contain id field' }
}
if (!('code' in obj) || typeof obj.code !== 'string') {
return { error: 'expected body to contain code field' }
}
if (!('type' in obj) || typeof obj.type !== 'string') {
return { error: 'expected body to contain type field' }
}
if (!('message' in obj) || typeof obj.message !== 'string') {
return { error: 'expected body to contain message field' }
}
return {
error: '',
value: new ErrorFrame( obj.id, obj.code, obj.type, obj.message)
}
}
}
This code almost works but I can't narrow object
into { id: unknown }
and I can't narrow { id: unknown }
into { id: string }
@DanielRosenwasser any thoughts on when this might be looked at by the TS team?
@Raynos you can create helper function like this
export type TypeNameMap = {
function: Function;
object: object | null;
string: string;
number: number;
boolean: boolean;
symbol: symbol;
bigint: bigint;
undefined: undefined | void;
};
export declare type TypeName = keyof TypeNameMap;
function has<K extends PropertyKey, T extends TypeName>(obj: any, key: K, type: T): obj is {[P in K]: TypeNameMap[T]} {
return (key in obj) && type === typeof obj[key];
}
class ErrorFrame {
constructor(
public id: string,
public code: string,
public type: string,
public message: string,
) { }
}
class Parser {
public validateBody(body: Buffer): { error: string, obj: unknown } {
let jsonBody;
try {
jsonBody = JSON.parse(body.toString());
} catch (err) {
return {error: "expected body to be json", obj: null};
}
return {error: "", obj: jsonBody};
}
public parse(body: Buffer): { error: string, value?: ErrorFrame } {
const {error, obj} = this.validateBody(body);
if (error) {
return {error};
}
if (typeof obj !== "object" || obj === null) {
return {error: "expected json body to be object"};
}
if (!has(obj, "id", "string")) {
return {error: "expected body to contain id field"};
}
if (!has(obj, "code", "string")) {
return {error: "expected body to contain code field"};
}
if (!has(obj, "type", "string")) {
return {error: "expected body to contain type field"};
}
if (!has(obj, "message", "string")) {
return {error: "expected body to contain message field"};
}
return {
error: "",
value: new ErrorFrame(obj.id, obj.code, obj.type, obj.message),
};
}
}
Also, this shouldn't error
const o = {
a: 1,
b: 2,
c: 3,
};
function f(s: string) {
if (s in o) {
console.log(s, o[s]);
}
}
f("");
f("a"); // a 1
f("b"); // b 2
f("c"); // c 3
f("d");
function check1(v: unknown): v is { name: string; age: number } {
if (typeof v !== 'object' || v === null) {
return false;
}
const a = v as { name: unknown; age: unknown };
if (typeof a.name !== 'string' || typeof a.age !== "number") {
return false;
}
return true;
}
function check2(v: unknown): v is { name: string; age: number } {
if (typeof v !== 'object' || v === null) {
return false;
}
if (typeof v.name !== 'string' || typeof v.age !== "number") {
return false;
}
return true;
}
This currently makes it impossible to write a type guard function that checks for properties without using casts or any
. This makes type guards a lot less safe because people often forget to check edge cases (e.g. in
will throw when used on undefined
, or if typeof x === 'object'
x
can still be null
). If we could declare the parameter as unknown
(instead of any
), it would force users to work their way through all edge cases with checks.
in operator case of simple map
const MAP = {
I: "info",
D: "debug",
W: "warn"
};
const t: string = 'I'
if (t in MAP)
MAP[t]// error
}
I've found myself floundering multiple times trying to write good type guards for unknown
inputs. Is there an official suggestion for what to do for now? I love the idea of an unknown
type, but it seems rather incomplete without a way to safely find out if properties exist on object
.
I've found myself floundering multiple times trying to write good type guards for
unknown
inputs. Is there an official suggestion for what to do for now? I love the idea of anunknown
type, but it seems rather incomplete without a way to safely find out if properties exist onobject
.
It can be a simple solution for some cases if you know the property name (also easily extensible).
function foo(obj: unknown, byDefault?: unknown) {
const { bar = byDefault } = { ...(typeof obj === 'object' ? obj : {}) };
return bar;
}
I've found myself floundering multiple times trying to write good type guards for
unknown
inputs. Is there an official suggestion for what to do for now? I love the idea of anunknown
type, but it seems rather incomplete without a way to safely find out if properties exist onobject
.
Here's one way. Check if the input can have arbitrary properties, and then verify their type.
interface DictionaryLike {
[index: string]: unknown;
}
export const isDictionaryLike = (candidate: unknown): candidate is DictionaryLike =>
typeof (candidate === 'object') && (candidate !== null);
interface Foo {
foo: string;
}
const isFoo = (candidate: unknown): candidate is Foo =>
isDictionaryLike(candidate) && (typeof candidate.foo === 'string');
Here's a workaround I found in this older issue, which seems to work sufficiently well for my use-case.
export function hasKey<K extends string>(k: K, o: {}): o is { [_ in K]: {} } {
return typeof o === 'object' && k in o
}
Also, here's the StackOverflow question I had about this, in case anyone wants to provide a better answer
@DanielRosenwasser would love to hear some followup from the TS Team, it would be really nice to have a working solution.
I'm on mobile but check the above design meeting notes when we changed the labels. We haven't been able to prioritize it recently, but are open to a PR.
I tried some of the solutions above but nothing really worked for me. The hasKey
snippet above for example will lose all type information except the one property. I wrote the following, and it seems to work in all the cases I've tried:
const hasProp = <O extends object, K extends PropertyKey>(obj: O, propKey: K): obj is O & { [key in K]: unknown } =>
propKey in obj;
If a property is known to be present then the type of that property is untouched, otherwise we add the property as unknown
.
@mkrause I have tried your code, because i am looking for a solution to my problem with Proxies. Unfortunatly i got an error, but i think it's more another TS problem than a problem with your code.
const hasProp = <O extends object, K extends PropertyKey>(
obj: O,
propKey: K,
): obj is O & { [key in K]: unknown } => propKey in obj;
const myObj = {
x: 'Hello',
};
const p = new Proxy(myObj, {
get: (target, key) => {
return hasProp(target, key) ? target[key] + ' World!' : 'nope';
},
});
I get the following error:
Type 'symbol' cannot be used as an index type.(2538)
Currently i am working with:
const hasKey = <T extends object>(obj: T, k: keyof any): k is keyof T =>
k in obj;
Which works in my Proxy problem. But here i have other issues that i cannot access the Property with string literals after the check.
@VSDekar Try the following:
const p = new Proxy(myObj, {
get: <K extends PropertyKey>(target : object, key : K) => {
return hasProp(target, key) ? target[key] + ' World!' : 'nope';
},
});
Basically, at the point you call hasProp
, key
is of some general PropertyKey
type, so extending the target
with a key of this type is not very useful. But you can "postpone" the type of key
using a generic, so that the check will be done at the usage site (p.x
) instead.
Agree unknown
is a very nice idea but I am struggling not to find it useless for most cases where I would want it, because this feature is missing. As-is, there's not a very typesafe way to do JSON ingestion in cases where the source schema can't be trusted (e.g. deserialization from storage after a schema has been updated, or processing user input).
I has been directed to this issue by typescript-eslint. I am sorry about has not spent a long time reading the conversations in this issue, but please tell me: should I fix below type guards or no, if those role is just check is unknown data is ECMAScript object or no.
function isNonNullObject(potentialObject: unknown): potentialObject is object {
return typeof potentialObject === "object" && potentialObject !== null;
}
function isNonEmptyObject(potentialObject: unknown): potentialObject is object {
if (typeof potentialObject !== "object" || potentialObject === null) {
return false;
}
return Object.entries(potentialObject as {[key: string]: unknown}).length > 0;
}
function isEmptyObject(potentialObject: unknown): potentialObject is object {
if (typeof potentialObject !== "object" || potentialObject === null) {
return false;
}
return Object.entries(potentialObject as {[key: string]: unknown}).length === 0;
}
What do you think of this? It supports and narrows union types.
function hasProp<O extends object, K extends PropertyKey>(
obj: O,
propKey: K
): obj is O extends unknown ? (K extends keyof O ? O : never) : never {
return propKey in obj;
}
glad to have a try, but what is the expected behavior on earth?
interface A {
common: string;
a: string
}
interface B {
common: string;
b: number;
}
type T = A | B;
function f(t:T){
const key:string;
if ('common' in t){
t; //T
}
if ('a' in t){
t; //A or A|(B&{a:unknown})?
}
if('c' in t){
t; // T&{c:unknown}?
if('d' in t){
t; // T&{c:unknown,d:unknown}?
}
t; // what is t?
}
if(key in t){
t; // what is t?
}
}
@ShuiRuTian I think @jcalz made the expected behaviour cases quite clear in his comments, little bit spread over the whole thread, but for the case you give:
What I want to see happen is:
- Only do the narrowing in #10485 if the type is a union where at least one constituent explicitly features the relevant property (edit: optional properties count as "explicit" also).
- Otherwise (or afterwards), have the
x in y
check be equivalent to the type guardy is typeof y & Record<typeof x, unknown>
(modulo the bug in #18538).
So for your example, it seems that the expected behaviour is:
interface A {
common: string;
a: string;
}
interface B {
common: string;
b: number;
}
type T = A | B;
function f(t: T) {
const key: string = 'foo'
if ("common" in t) {
t; //T
}
if ("a" in t) {
t; //A
}
if ("c" in t) {
t; // T&{c:unknown}
if ("d" in t) {
t; // T&{c:unknown,d:unknown}
}
t; // T&{c:unknown}
}
if (key in t) {
t; // T
}
}
And another example:
interface Foo {
foo: unknown;
}
function parseFoo(value: unknown): Foo {
assert(typeof value === "object");
assert(value != null);
// desired: assert('foo' in value);
assert(isIn("foo", value));
return value; // object & {foo: unknown}
}
export function isIn<Key extends PropertyKey, T>(
key: Key,
value: T
): value is T & { [P in Key]: unknown } {
return key in value;
}
export function assert(expr: unknown, msg = ""): asserts expr {
if (!expr) throw new Error(msg);
}
Whoops, I did not notice it. @kasperpeulen thanks for pointing it and give out the answer. If so, it seems not very hard to give a PR, but I still have a question about advanced situation. Anyway, let us talk about it after the beta version. Thanks for the kind help again!
OK, we have got the beta version now. Welcome to have a try. And I add some extra rules.
this/any/unknown
, nothing would happen.{propName : unknown}
Is there other suggestion? Or something I missed again?
The helper function can just be:
export function has<P extends PropertyKey>(target: object, property: P): target is { [K in P]: unknown } {
// The `in` operator throws a `TypeError` for non-object values.
return property in target;
}
Which is also the behaviour of Reflect.has
.
TypeScript Version: 2.8.0-dev.20180204
Search Terms:
in
operator type guard generic assertCode
Actual behavior: The compiler does not recognize that the objects have relevant keys even after checking for the existence of the key with the
in
operator. According to a comment by @sandersn, thein
type guard (as implemented in #15256) narrows by eliminating members from a union; it does not assert that a key exists.Desired behavior: The compiler would assert that each object had a property with the relevant key, after having checked for the existence of the key with the
in
operator. Note that one possible implementation of an asserting type guard would look likebut this does not behave exactly as desired, possibly due to the bug in #18538:(not sure if #18538 was officially fixed, but it's not erroring anymore)If a fix for #18538 appears and makes theg()
function compile without error, great. Otherwise, maybe the property assertion forin
could happen some other way. Not sure.Playground Link: Here
Related Issues:
10485, Treat
in
operator as type guard15256, Add type guard for
in
keyword18538, Error when mixing
keyof
and intersection type and type variable (fixed?)(EDIT: #18538 seems to be fixed) (EDIT: change
any
tounknown
)