DEV Community

Vincent Jaubert
Vincent Jaubert

Posted on

"in" operator considered harmful in Typescript

Let say you have two types of users in your app, regular users and superUsers.

type RegularUser = {
  id: string;
  username: string;
  email: string;
}

type SuperUser = {
  id: string;
  username: string;
  email: string;
  isSuperUser: true; //for demonstration only, don't do this
}

Enter fullscreen mode Exit fullscreen mode

You then sum them in a union type User

type User = RegularUser | SuperUser;
Enter fullscreen mode Exit fullscreen mode

Now, when you receive a User, how do you discriminate between RegularUser and SuperUser ?

function isSuperUser(user: User){
  return user.isSuperUser; //error
}
Enter fullscreen mode Exit fullscreen mode

That won't work as TS doesn't allow to access properties that are not present on all types of a union type.

The usually recommended way is to use the in Javascript operator.

function isSuperUser(user: User){
  return "isSuperUser" in user;
}
Enter fullscreen mode Exit fullscreen mode

The in operator ensure at runtime that user has the isSuperUser property.

One interesting Typescript refinement (which in this case is called narrowing), is that the compiler is clever enough to know which member of the union we have, in the part of the code garded by the in check.


function regularUserAction(user: RegularUser){
  //do something with RegularUser
}

function superUserAction(user: SuperUser){
  //do something with SuperUser
}

function userTypeActionSwitching(user: User){
  if ("isSuperUser" in user){
    //user is narrowed to SuperUser
    superUserAction(user);//compiler happy
  } else {
    //user is narrowed to RegularUser
    userAction(user);
  }
}
Enter fullscreen mode Exit fullscreen mode

End of story ? I don't think so.

Let say, we now decide to refactor our code and use a tagged union (also called discriminated union) instead.
Good idea !

type RegularUser = {
  id: string;
  name: string;
  email: string;
  type: "regular";//here is the tag 
}

type SuperUser = {
  id: string;
  name: string;
  email: string;
  type: "super";//another tag
}
Enter fullscreen mode Exit fullscreen mode

We will probably have to rewrite parts of our code to take into account those changes. Fortunately we used a type system to ensure that we didn't forget anything. We will see a lot of red lines in our editor that will prevent to ship bogus code, right ?
Wrong !

Let's look at the consumer of our User union

function isSuperUser(user: User){
  return "isSuperUser" in user;//No Error !
}
Enter fullscreen mode Exit fullscreen mode

Although neither RegularUser nor SuperUser has an isSuperUser field anymore, the compiler doesn't mind.

Why is that ?

What is happening with the in operator is that it gives us the warranty that the code running after the check will have the tested property.

function getUserName(user: User){
    if ("userName" in user){
        return user.userName; //no error reported
    }
    return "";
}
Enter fullscreen mode Exit fullscreen mode

Here, altough we made a typo writing userName instead of username, the compiler won't complain. If user ends up not having a userName field at runtime, there won't be a problem since the first return will never run.
However, this is probably not what we want, and we wrote dead code that will never run.
The Typescript Langage Service (the software that is providing our editors with data enabling autocomplete) is even fooling us by showing a userName entry while typing. For him there is a userName field, since we asserted it with the in check.

We all know that type assertions which we sometimes have to use, with the as operator, are dangerous.

But while type assertions doesn't allow everything

function wontWork(){
    const obj = {
        foo: "foo",
        bar: "bar"
    } as {
        foo: string,
        baz: string //baz instead of bar ,compiler will complain
    }
    return obj.baz;
}
Enter fullscreen mode Exit fullscreen mode

With the in operator, the compiler doesn't care

function willWork(){
    const obj = {
        foo: "foo",
        bar: "bar"
    };
    if ("baz" in obj){ //baz instead of bar, but no error
        return obj.baz;
    }
}
Enter fullscreen mode Exit fullscreen mode

Using the in operator is like using the dreaded as unknown as

function willWorkButDangerous(){
    const obj = {
        foo: "foo",
        bar: "bar"
    } as unknown as {  
        foo: string,
        baz: string
    };
    return obj.baz; //securities disabled, no error
}
Enter fullscreen mode Exit fullscreen mode

As with any, you know that when you use as unknown, as, you are on your own.

Were you aware that it was the same with in ?

Conclusion

My opinion, which you are welcome to challenge, is that the in operator is an anti-pattern in Typescript.

It is true that code which is guarded by a in operator won't be able to run if in checks for property that doesn't exist.

But it can lead to buggy programs with parts of the code that end up being dead without the compiler warning us.

This is not a mistake by the TS team since Javascript and Typescript have a structural (or duck typed) type system which guarantees lower bound (presence of properties) but not upper bound (no superfluous properties) correctness.

Top comments (20)

Collapse
 
zirkelc profile image
Chris Cook

I have to disagree with you that the in operator is an anti-pattern in Typescript.

The in operator checks at runtime if the property is available somewhere in the property chain. TypeScript correctly infers the type of user.userName as unknown. However, if you add a return type to the getUserName() function, you will get the expected type error:

TypeScript Playground

function getUserName(user: User): string {
    if ("userName" in user){
      //> error: Type 'unknown' is not assignable to type 'string'.(2322)
        return user.userName; 
    }
    return "";
}
Enter fullscreen mode Exit fullscreen mode

In my opinion, if you rely on types, you should completely type input and output, especially if you use dynamic accessors like in or Object.hasOwn().

Collapse
 
dannymcgee profile image
Danny McGee

I personally prefer to omit return-type annotations and let TS infer the correct type when possible β€” at times, this even yields more correct results, because the return type annotation can act as a sort of type-cast under certain circumstances. Without the return type, you would still get the expected error as soon as you tried to actually do anything useful with the returned value:

// ...
const users: { [key: string]: User } = {};
const user: User = {
    type: "regular",
    username: "foo",
    email: "foo@foo.com",
    id: "foo",
}

const userName = getUserName(user);
if (userName in users) {
//  ^^^^^^^^ Error: 'userName' is of type 'unknown'.
    console.log(users[userName]);
//                    ^^^^^^^^ Error: Type 'unknown' cannot be used as an index type.
}
Enter fullscreen mode Exit fullscreen mode
Collapse
 
stretch0 profile image
Andrew McCallum

This is an excellent point you raise.

I think this is a strong example of why using explicit return types is good.

There is more room for error with implicit return types.

Collapse
 
ctsstc profile image
Cody Swartz

πŸ’― on return types. Your consumers should not be what enforces and validates your contact, but rather be explicit of what it's expected to return.

Maybe your assumption is that the function should return void if the property wasn't found. If consumers are utilizing this then this is valid code.

Exhaustive unit tests, and code reviews are the winners in this case, especially a typo too. If you wrote 100% test coverage there's a logical branch in the code which returns void. If you write the test covering that scenario to get 100% coverage, then you've either realized your code doesn't do what you want, or you've now documented that it's expected behavior.

Collapse
 
joshuakb2 profile image
Joshua Baker

Although your example is correct, Typescript's inference with the "in" operator is sometimes unsound. Here's a modified playground to demonstrate what I mean: Playground

So it's important to be aware of the danger of "in" in Typescript, but it's only dangerous in certain circumstances.

Collapse
 
zirkelc profile image
Chris Cook • Edited

Sorry, I can't follow why your example should be wrong:

You defined the isSuperUser as type true and you checked if it exists on the given object with the in operator. TypeScript infers the type as true (because it exists) and lets you call toString() on it.

That all sounds right to me. Why do you think the type should be unknown in this case?

Collapse
 
artxe2 profile image
Yeom suyun

I think the best way to fix the first example is to add isSuperUser?: false to RegularUser.
compiler does not complain about isSuperUser, and we can see that newly added roles should have isSuperUser if we add a new role to the User type without isSuperUser, the code will emit an error.

Collapse
 
vjau profile image
Vincent Jaubert

Thank you for your comment.
Why would you make the isSuperUser optional on the RegularUser ?

Collapse
 
artxe2 profile image
Yeom suyun

From a business perspective, I would use a structure like roles: ("ADMIN"|"GUEST")[] for permission checking.
However, from a type perspective, flags like isXX are not a problem.

Collapse
 
jdthegeek profile image
John Dennehy

This was my thought too.

Even if we REALLY needed separate types, I would possibly still add 'isSuperUser: Never' to the User type or something similar.

Collapse
 
etienneburdet profile image
Etienne Burdet • Edited

I agree in can be confusing for type assertion in TS and I don't think I use it at all these days. The code you write isn't wrong in itself, it's just a bad assertion in the first place. But with other patterns than in TS can warn you better indeed.

Something like :

Β 

function isSuperUser(user: User): user is SuperUser{
  return user.type === 'super'
}
Enter fullscreen mode Exit fullscreen mode

Will yield much better safety indeed.

Collapse
 
scorbiclife profile image
ScorbicLife • Edited

I think it would be nice if you could elaborate that the reason why the following compiles

function getUserName(user: User){
    if ("userName" in user){
        return user.userName; //no error reported
    }
    return "";
}
Enter fullscreen mode Exit fullscreen mode

is because you can pass this into getUserName

const userWithUserName = {
  id: "0",
  name: "Admin I. Strator",
  email: "foo@bar.com",
  type: "super",
  userName: "root",
} as const satisfies SuperUser;
const name = getUserName(userWithUserName); // "root"
Enter fullscreen mode Exit fullscreen mode

Which is valid because, as you mentioned, typescript types are structurally typed. (Edited for clarity)

Collapse
 
vjau profile image
Vincent Jaubert

Thank you for your comment.
Doesn't it make the article more complicated to grasp ?
I just mentioned structural typing so the reader understand this is not necessarily a design mistake, and most typescripters already knows about structural typing or can easily learn about it from other sources.

Collapse
 
scorbiclife profile image
ScorbicLife • Edited

I thought it would be nice to know upfront what's happening exactly and that this is not a bug in TypeScript, but I guess it could be overwhelming for a subset of the target audience. Thanks for the response.

Collapse
 
dannymcgee profile image
Danny McGee

You make some good points here, but I think it's incorrect to conflate the in operator with arbitrary type casting. To use a contrived example, TypeScript will accept this code, but it will "crash" your JavaScript application:

type Foo = { userName: string };
const foo = {};
console.log((foo as unknown as Foo).userName.toUpperCase());
Enter fullscreen mode Exit fullscreen mode

This would be the "equivalent" code with an in check, but TypeScript will not accept it as valid:

type Foo = { userName: string };
const foo = {};
if ("userName" in foo) {
  console.log(foo.userName.toUpperCase());
  //          ^^^^^^^^^^^^ Error: 'foo.userName' is of type 'unknown'.
}
Enter fullscreen mode Exit fullscreen mode

You would need to add an additional runtime check for TypeScript to be happy with it:

type Foo = { userName: string };
const foo = {};
if ("userName" in foo && typeof foo.userName === "string") {
  console.log(foo.userName.toUpperCase());
}
Enter fullscreen mode Exit fullscreen mode

Now, that is still misleading code β€” the block will never be entered, because it's impossible for foo to have a userName property β€” but that's really not the same as causing your application to crash.

The fact is that JavaScript is a dynamically typed language, and sometimes in the real world we will encounter keys on objects that our types haven't fully accounted for (especially when depending on third-party JavaScript modules). But TS type annotations and runtime checks like if (<key> in <object>) <...> are fundamentally different things β€” TypeScript provides us some compile-time safety (and a nice developer experience), but as a static analysis tool, it's possible for its assumptions to be incorrect. It's not possible for an if (...) block with a falsey condition to be entered at runtime, so the only thing TypeScript is doing here is updating its assumptions to reflect the reality that the runtime check has established.

Collapse
 
mickmister profile image
Michael Kochell

What you change the function signature from

function isSuperUser(user: User){
return "isSuperUser" in user;
}

to

function isSuperUser(user: User): user is SuperUser {
return "isSuperUser" in user;
}

Collapse
 
ctsstc profile image
Cody Swartz

Enabling explicit return types, exhaustive unit tests, and code reviews are the winners in this case, especially a typo too.

Even without exploring return types, if you wrote 100% test coverage there's a logical branch in the code which returns void that you would have to test. If you write the test covering that scenario to get 100% coverage, then you've either realized your code doesn't do what you want, or you've now documented that it's expected behavior.

Collapse
 
sirmoustache profile image
SirMoustache

The answer here is unit tests. TypeScript is covering a lot of cases, but not all.

Collapse
 
dsaga profile image
Dusan Petkovic

Haven't heard of the structural type reference, learned something useful today!

btw. isn't the whole point for the code that is guarded by the inoperator not to get executed?

Collapse
 
cabbage profile image
cab

And it isn’t. OP is making the point that a typo can mess up your intention and produces unreachable code.