Enhancement: [ban-types] Split the {} ban into a separate, better-phrased rule

[JoshuaKGoldberg]

Before You File a Proposal Please Confirm You Have Done The Following...

• I have searched for related issues and found none that match my proposal.
• I have searched the current rule list and found no rules that match my proposal.
• I have read the FAQ and my problem is not listed.

My proposal is suitable for this project

• I believe my proposal would be useful to the broader TypeScript community (meaning it is not a niche proposal).

Link to the rule's documentation

https://typescript-eslint.io/rules/ban-types/

Description

Filing this issue as a followup/superset of the more targeted #8697. As of microsoft/TypeScript#49119, TypeScript has much better handling of {} semantics than it did when the ban-types default options were written. @RyanCavanaugh -the dev lead for TypeScript- called out in microsoft/TypeScript#57735 (comment) that as it stands today, { } is a valid type with a valid meaning in TypeScript.

Proposal: let's re-think the default options in ban-types to agree with the way TypeScript now works?

My first proposal would be, roughly...

• Remove any bans of {} altogether, including in recommended and strict
• Add a ban on NonNullable<unknown> that suggests switching to {}

...but I'd want to hear from @bradzacher and @RyanCavanaugh on whether that satisfies the intents of both TypeScript and ban-types.

Fail

let nonNullish: Object;

Pass

let nonNullish: {};

Additional Info

For clarity, the intent of the two issues I'm filing are:

• This one: holistically rethinking ban-types, which might take some time
• Enhancement: [ban-types] Allow {} in the recommended config #8697: spot improving the recommended experience, which can land first

If this issue is resolved before #8697, that's great too.

[bradzacher]

{ } is a valid type with a valid meaning

To be very clear here - the ban does not exist because we think it's an invalid type. And that statement alone shows that likely Ryan is reacting to peoe sending snippets to him, rather than the full docs or full error message.

To make things absolutely clear - this is error message that users see when they use {}:

`{}` actually means "any non-nullish value".
- If you want a type meaning "any object", you probably want `object` instead.
- If you want a type meaning "any value", you probably want `unknown` instead.
- If you want a type meaning "empty object", you probably want `Record<string, never>` instead.
- If you really want a type meaning "any non-nullish value", you probably want `NonNullable<unknown>` instead.

We don't just ban it outright - this message has been very carefully constructed to lay all of the cards on the table and let the user make an informed decision.

Additionally we don't ban it because we don't understand TS here and that we've labelled the type as invalid. No way no how. Instead the reason that we have a ban on {} is because it doesn't mean what people think it means.

People think it mens "an object with no properties" but it doesn't mean that at all. So the ban exists to warn people of this fact. People can easily disable comment if they really did mean "any non-null value" - but also IMO the actual cases where you mean that are 1 in a million.

Put another way - when the average user writes type Foo = {}; they mean "an object with no properties" - they do not mean "any non-null value".

So I'm a strong, strong 👎 to this.

[bradzacher]

I'd be happy for this be split out into a separate rule entirely - as long as it was in the recommended set still.

I firmly disagree that it's incorrect to flag {} for the vast, vast majority of users because of the misconception.

There are some cases where we could not flag it because it does end up meaning "the right thing" (eg in an intersection type). But we can't add such handling into ban-types.

[RyanCavanaugh]

I still don't agree with this and it's still a constant source of frustration for me. It's still common for people to show up with one of these workaround types, complaining that they don't do what they want, when the thing they were using correctly in the first place actually was { }.

Lint rules are not a substitute for reading documentation, and they shouldn't try to be, either. They're there to prevent mistakes and enforce consistent style.

It makes a lot of sense to ban String, for example. That's a great rule. It's an easy mistake to make. It's extraordinarily rare to write String and not mean string; in "good" user code this happens effectively 0% of the time. If you really do mean String, there should be a justification for it.

People think it means "an object with no properties" but it doesn't mean that at all.

By this logic, we should also ban { length: number } because it doesn't mean "an object with only a length property". If it's a valid source of unfixable confusion for 0 properties, it must also be a source of unfixable confusion for 1 property, and object types altogether should be a lint violation.

But you're not solving the underlying problem of users not knowing what { } means (as evidenced by the many people coming to our repo citing this lint rule of why we're using it wrong!). There are many, many useful places where { } can and should appear in legitimate user code. It has a meaningful place in the hierarchy of types.

Instead the reason that we have a ban on {} is because it doesn't mean what people think it means

Banning { } because it doesn't mean "a completely empty object" is like banning Math.cos because some users might think it takes degrees when it actually takes radians. The existence of some uneducated users doesn't justify an outright prohibition on it.

I don't think it makes sense to see code littered with lint suppressions that just say "The person who wrote this code has read the docs", and the only workaround that should be recommended is NonNullable<unknown>, because the other types recommended don't even do what they say they do.

People can easily disable comment if they really did mean "any non-null value" - but also IMO the actual cases where you mean that are 1 in a million.

I don't understand this when one of the proposed workarounds is to use a type alias that itself uses { }. If it's 1 in a million, why are you recommending NonNullable<unknown>? People are going to read the definition of that alias, see it uses { }, and remain as confused as when they started. If it's not actually uncommon, then let people write the type that means the thing they want it to mean.

[RyanCavanaugh]

I get that the confusion can exist, but also, you're going to see { } appear in your code regardless. Someone looking at this

seems unlikely to conclude "Wow, TypeScript wrongly inferred that p is exactly an empty object".

I just fundamentally don't agree with the idea that a type TypeScript will speak back to you ex nihilo, in uninteresting idiomatic code, is really so confusing that it has to be banned outright. People writing TS will see { }. They'll have to understand what it means. Unlike String, you're still going to see { } in other places - from TS itself, from well-written definition files, from the documentation itself. It can't be the guiding principle for how some syntax is allowed and other syntax gets shunned in favor of an alias for a thing you're still going to be seeing.

Orienting the default lint experience around a user who hasn't used TS for more than a few days is the wrong mindset to have when making this ruleset.

It'd be bad to ban void just because some people think it means undefined.

It'd be bad to ban (x: string) => unknown in type syntax because some people think it means "must be an arrow function".

It'd be bad to ban let x: SomeClass because some people think it means the SomeClass constructor rather than the instance.

I could go on for days; users will misapprehend themselves of practically anything on even the most straightforward syntax.

[Josh-Cena]

I'm in agreement with @RyanCavanaugh here. {} has no soundness issues and users won't run into any problems even if they are under the false impression that {} "only accepts empty objects", other than more values than they expect being accepted—but why shouldn't they be accepted anyway?

[bradzacher]

My personal experience working in the industry is that people do not read the TS handbook and they do not fully understand structural typing.

Many people just start using TS and "figure it out". Often times people aren't afforded the time to read the docs and are instead thrown in the deep end and learn by doing. This isn't just true of TS's docs - the same goes for react docs, angular docs, internal company docs, etc etc etc.

There are also a lot of people that started back in the day when the TS docs weren't as good as they are today as well. They often don't know the docs have changed and thus haven't updated themselves.

I don't think it's fair to say "you must RTFM and there's no valid replacement for RTFM". Some people aren't able to learn by reading such walls of text. Some people don't have the time or energy to assign to reading such prose.

Lint rules are not a substitute for reading documentation, and they shouldn't try to be, either.

I fundamentally and whole-heartedly disagree. I could not disagree more. I have a lot of experience working directly with large bodies of engineers to try and teach them - it's been a substantial portion of what I've been doing for the last 5 years.

This applies to all docs as I mentioned before and is not unique to programming language docs.
If you write a doc - few people will read it.
You can spam it on whatever internal channels people use but there will be copious amounts of peolle that won't read it. Either they miss the doc or they see it and bookmark it and forget to get back to it or they see it and ignore it. The result is the same - they don't read it.
You can organise a brownbag talk or a frontend forum or a standup or a sitdown or a soapbox or whatever you want to call it and talk to people with examples and people will see it. But you have the same problem - some people listen, some people don't. Some people attend, some people don't.

No matter what you do - requiring people to RTFM is just not a viable way to disseminate and saturate information amongst your engineering cohort. In order to properly teach people the only way to do so is to do it live in the one place you have their undivided attention - in their IDE. The linter is ultimately the only tool that exists to do this. It's the only tool that's guaranteed to be integrated into an engineer's coding flow.

[Josh-Cena]

Even when people don't read documentation and assume {} means "empty object"—how could it become a footgun for them?

[bradzacher]

@Josh-Cena {} allows assignment of number | string | boolean in addition to any other object type.

So if you're using it to mean "an OBJECT with exactly zero properties" you may be quite surprised to find that someone has passed a number to you.

As a real example - it's not uncommon for backends to accept an empty object as a way of signalling true (instead of a boolean) in order to allow future extensibility of an option (eg it's easier to add properties to an object than it is to migrate callers from a boolean to an object -- one is a major breaking change and one is a simple minor change).

So if you type your backend payload as { prop: {} } then you might think "cool everyone needs to send an object with zero properties to my backend and my backend parser will be able to deserialise this JSON". And then someone sends your backend { prop: 1 } and 💥 the request crashes because the deserializer did not expect a number.

This example is made particularly unsafe due to the across-boundaries passing of data, but one can imagine cases where people write code that assumes specific properties based on an assumption of "this thing must be an object with zero properties".

It's an incorrect assumption and the lint rule is attempting to inform people that their assumption is wrong here.

And to be clear - that's all it's doing. The error message makes ZERO mention of the type being invalid, unsafe, unsound or anything. It just says "{} actually means "any non-nullish value".". It's purely there to challenge the incorrect assumption.

[bradzacher]

I would also like to make it clear that whilst this is the default of the rule - the rule is completely reconfigurable. People can very easily turn off just this specific error message with this config:

{
  "@typescript-eslint/ban-types": ["error", {
    "extendDefaults": true, 
    "types": {
      "{}": false
    }
  }],
}

playground.

We even explicitly show this case in the rule's docs.

But people purposely choose NOT to reconfigure the rule. A lot of this is likely laziness as people don't want to bother changing their config. But I personally think that a lot of this is also because people agree with the error for all the reasons I've stated.

[Josh-Cena]

Hmmm. Should { toString: () => string } or { length: number } also be banned, then? Backend "only allowing an empty object" still sounds artificial to me.

I'd be happier if this is split out as a separate rule... It's informative but disruptive to most workflows, especially when developers can't easily modify the lint config.

[bradzacher]

Should { toString: () => string } or { length: number } also be banned, then?

From my experience these types are actually fine.

It's been a very very rare case that I've seen someone write { toString: () => string } - however when I have seen it the intent has always been "some value that I can stringify". So there's no misalignment between "intent" and "reality" there.

Put another way - the intent of { toString: () => string } is that you want to call value.toString() - which works regardless of the value type that's passed (object, number or otherwise). So the intent clearly aligns with reality.

Scanning through some of the usages of this type in open source (of which there are very few) it looks like what I said is correct - the intent is to have some stringifiable value.

I've never personally seen { length: number } used myself and there are sparse few usages in open source. A lot of usecases seem to be within JSX component functions wherein structural typing doesn't apply. I don't know the intent of such types outside JSX so I can't really comment further here.

[bradzacher]

One thing that I think needs to be done here is to separate the power users from the average users.

Power users are 110% on board with how structural typing works and how things. Everyone in this thread is a power user - Ryan quintuply so. There are two outs for such power users to step out of this behaviour: either disable comments or reconfiguring the rule. I'm always partial to the former because I like to explicitly document things so that future non-power-users can understand why something is safe.

For example instead of disabling the {} behaviour because I wrote code like this:

type T<U> = { prop: string } & U extends SomeType ? { other: number } : {};

I would instead prefer to write:

type T<U> = { prop: string } & U extends SomeType ? { other: number } : 
  // eslint-disable-next-line @typescript-eslint/ban-types -- this usage of {} does work as an "empty object type" as it is used in an intersection
  {};

But I understand that in this regard I'm unique and most power users don't like to be so verbose because they believe that such a thing is obvious.

---

For the average user - they likely aren't completely on board with structural typing.

From my personal experience teaching people TS - it takes a lot to truly make them understand this or more importantly REMEMBER IT.
I've seen countless bugs in code caused because even TS experts forget that structural typing exists. I've seen even more countless cases of people passing unused properties around because someone deleted a property and assumed that TS would catch it - forgetting about structural typing.

Again - I spent a not insignificant portion of my job supporting other engineers. So often the answer to someone's question of "why didn't this do what I expected" was "because you didn't expect structural typing".

Ryan you said "Orienting the default lint experience around a user who hasn't used TS for more than a few days is the wrong mindset to have when making this ruleset." but from my experience - people forget about structural typing! In so much code structural typing either doesn't really do anything or it does "what they expected" by acting as an inexact assignment. It's the rare cases where someone butts up against it that they are reminded of its existence. Which is a good thing - most people don't even need to consider structural typing most of the time!

For the average user having a reminder that says "hey this thing probably isn't what you think it is" is a really good thing.

But why special case {} above all others? Because it's so so so common to write {} and try to mean "an object with zero properties". It's many times more common than other types which allow assignment of non-object values.

[bradzacher]

I would also like to make clear here that I'm not married to the wording of the error or its suggestions. If it's seen as an unclear message - we can change this now in a minor version change.

This wording is verbose and could be made more verbose. If we think we can clarify it further then LET'S DO IT.

[Josh-Cena]

Best option for me: a separate rule.

Second best: further clarify in the message that "if you actually intend to do this don't be afraid to add a disable comment or disable this particular type ban"

But, we should definitely turn it off in recommended because it sounds too opinionated.

[bradzacher]

seems unlikely to conclude "Wow, TypeScript wrongly inferred that p is exactly an empty object".

I would like to say here that this code is very much a code golf strawman example. By-and-large people do not ever see a {} narrowed from some other type by TS.

From my experience if someone is in an unknown state they are much, much more likely to write code like this which narrows to object, not to {}.

if (typeof p === 'object' && p !== null) {
   p;
// ^? const p: object
}

I don't understand this when one of the proposed workarounds is to use a type alias that itself uses { }.

Because from my experience few people actually "go to definition" on the TS builtin types to read their implementations. Additionally it wasn't until recently that this type itself used {} - so a lot of people would have likely seen the old conditional type form as opposed to the current & {} form.

But that's beside the point - I don't quite understand why the implementation of NonNullable matters in regards to the error message. Because the final type that comes out will actually be unknown & {} - not just {} like the user originally wrote. So we're not recommending a like-for-like - we're recommending an entirely different type.

If it's 1 in a million, why are you recommending NonNullable<unknown>?

Because we're being complete. People complained in the past that they had usecase X, Y and Z that the error message didn't account for and they were upset that the error just said "dont use this type". So we changed the message to be a lot more verbose and attempt to act as documentation and cover all of the bases. We tried to enumerate all the usecases so that when the rule reported then people could understand the error and then remediate the error.

But again - if you think there's more to be said as I mentioned lets make it better.

[bradzacher]

Ryan I fully empathise with your support pain. Our intent is definitely not to push users to the typescript repo to complain that there's some weird behaviour and the linter doesn't align with it.

You have an understandably negative sentiment towards this suggestion because it causes you support burden and you're sick of correcting users' misunderstanding of the message and the type.

We can and should be doing better here. We should not be causing pain for you guys unintentionally.

---

Though I would say that there's likely actions from both sides here.

As an example the ban-types rule used to disallow object because many people complained that it was really hard to use. Then support for in refinements was added and that fixed all user complains with the type. It ultimately resulted in a win for everyone - by understanding the shortcomings that lead to people banning the type we were able to advance the state of the world for the better.

The fact that there's user want [1] for banning {} shows there's a gap on the TS side. I don't know what the remediation is, but there is probably something. As a naive example - is there a space for an "exact empty object" type so that users can explicitly declare such a thing?

Similarly there's a gap on our side - we can improve the error message for people so that they don't misunderstand it and want to file issues in TS.

[1] I say user want because banning {} didn't ship with the first version of the rule! It was asked for and added by users. It wasn't the typescript-eslint team that drove this!

[rubiesonthesky]

As a typescript user, and specifically when I was new user, {} looked like it would behave similarly as a value and as a type. Same for [].

I think the problem may lay somewhere that it’s not clear what {} means. You can read it from documentation and Google of course. But I think it can be a paid point to try to find how do you type empty object, how do you type object JSON style object that can have any keys, and how do you type what {} actually means.

It’s too much for a lint rule, but I think that in bigger code bases having type aliasing spirit of ObjectLike = {} could help. Then you could just disable a lint for that row and everybody would know when to use what (one can hope).

—-

I agree with everybody here. I think separate rule could be good. And maybe it should not be in recommended. But then, this is common mistake to make, so it would be good to be in recommended set.

I think those types in typescript language are just little bit unfortunate. They are correct but I feel like they are technically correct. And maybe this time that is not the best kind of correct.

[JoshuaKGoldberg]

I've read through this & the related threads a few times and I think the banning of {} by default in recommended isn't likely worth the user pain. +1 that {} is a type that commonly confuses people and there should be some warnings around improper usage of it. But it's a core part of the TypeScript language now - I think it would be ideal to lint for proper usage around it, not against using it at all.

the rule is completely reconfigurable

...but, almost no users do. So it's still a pain point even if users are "in the wrong".

Lint rules are not a substitute for reading documentation, and they shouldn't try to be, either.

I interpreted this as implying the two should work together, not that they're mutually exclusive. +1 to that. Documentation inherently cannot capture all the same nuanced things that linting does. If it tried, it would become so deep that nobody would be able to read it. So, +1 to the last couple paragraphs in #8700 (comment).

I don't quite understand why the implementation of NonNullable matters in regards to the error message.

I think this matters quite a lot: many devs actually do read the built-in types. Personally I reference the files in my Learning TypeScript book (Chapter 11: Declaration Files > Library Declarations) and generally recommend folks read them as part of learning TypeScript when I teach it / give my workshops. I believe learning them is practically required to truly know the type system to learn them.

By banning something that's used in a valid way in the built-in types, we add an additional complication to learning TypeScript well. We're saying "well, this is a bad practice, except in certain scenarios, which is used but not explained in the built-in types". Even if that's correct in some ways, it's confusing. We're now seeing a byproduct of that confusion in that people are reporting issues on TypeScript.

---

Taking a step back: I think there are really two uses for the {} that are being discussed here:

• 🍎 Declaration: using {} in a type, like let shouldTakeAnyObject: {}
• 🍌 Value assignment: providing a non-object value to a {} type, like shouldTakeAnyObject = "oops"

Right now we're using ban-types to ban 🍎 declarations. But I think the root intent is that we really want to ban 🍌 value assignments. Which leans me heavily towards a separate rule.

Proposal...

• Non-breaking changes:
  • Reword the phrasing of 🍎 ban-types to be more explicit (I think we're all in agreement there?)
  • Add a new rule in strict that bans 🍌 value assignments to locations of type {}
• Breaking changes for v8:
  • Remove {} from the default options for 🍎 ban-types
  • Promote that new 🍌 value assignments rule to recommended

Would this satisfy everyone?

[ehoogeveen-medweb]

Regarding the 🍎 example above, let shouldTakeAnyObject: {} is still bad practice, right? The declaration doesn't say "should take any object", it says "should take anything other than null and undefined". If you want to allow any object (but not null) it should be let shouldTakeAnyObject: object (though that still allows things like arrays and functions).

[JoshuaKGoldberg]

@ehoogeveen-medweb that's what I was trying to imply with the name 😄 that the theoretical developer meant for it to be object.

[Josh-Cena]

If a library declares a function taking {} and they specifically meant "anything non-null" (for whatever reason), how are we doing any service to the library user by reporting assignments to it?

In general I really don't know why users want to treat primitives so differently from objects, when such "object" cannot have any known properties anyway. The only difference would be (a) JSON.stringify (as in Brad's example) (b) typeof (c) other APIs that do runtime type checking. All other things can and/or will behave the same.

[JoshuaKGoldberg]

and they specifically meant "anything non-null" (for whatever reason) ... how are we doing any service to the library user by reporting assignments to it?

Hmm, yeah, I suppose this goes against the spirit of the {} type's intent. It's a little frustrating that what we're trying to do (help users avoid accidental {} with literals/primitives) is inherently what that type is made to allow. There's no universal good way to know whether the library intended "anything non-null" ({}) or "any object" (object, but they used {} instead by accident).

[ssalbdivad]

Just wanted to add my two cents that I've seen multiple teams I've consulted for get confused by the {} lint rule and incorrectly substitute stuff like Record<PropertyKey, never> or one of the other recommended types instead.

This ends up introducing a bunch of weird index signatures into what should be simple mapped types. At a type-level, {} is the correct empty type for mapping over.

Code-bases that lean heavily into mapped types are also much more likely to use @typescript-eslint (all of those I've worked on have), but most take the advice at face value even in these situations and end up making their types much less efficient and/or incorrect as a result.

I agree with @RyanCavanaugh that {} is fundamental to the language for an intermediate TS dev, and it seems hard to imagine a significant number of cases where it would occur to a beginner (or anyone really) to implement some API that expects exactly an empty object.

I would be very happy to see this toggled off by default so the focus of the rule can be shifted back toward types like any that are easier to complain about in a lint rule and more likely to reflect a real problem.

[JoshuaKGoldberg]

@bradzacher, @Josh-Cena, @kirkwaiblinger, and I discussed internally. We're not all in complete agreement about #8697's moving the {} ban out of the recommended config (last call was two in favor, two against).

But, a compromise we're happy with for now is splitting out the ban into a new rule. This would give two benefits:

• Make it easier to have a single dedicated place that explains the nuanced issues with {}/Object
• Make it easier for folks to have a single rule to disable if they don't want it

Renaming this issue to indicate it's for specifically the splitting {}/Object banning out into a new rule. Adding to the v8 milestone as removing that functionality from the default ban-types is a breaking change.

Note that this does mean we plan to keep the {} and Object ban in the default recommended ruleset in typescript-eslint@v8. We understand the downsides of the ban: that there are legitimate cases for {} and it causes user+maintainer pain on the TypeScript side how folks come away from its reports confused about objects. Our team consensus is that the upside of the ban is that many users get educated about structural typing and avoid common mistakes around {}. We'd like to see how the moving of the ban into a better-documented rule with better report phrasing impacts user behavior before taking the big step of removing the ban.

[JoshuaKGoldberg]

Code-bases that lean heavily into mapped types are also much more likely to use @typescript-eslint (all of those I've worked on have), but most take the advice at face value even in these situations and end up making their types much less efficient and/or incorrect as a result.

Indeed. I hate this 😄. It's super incorrect+wrong behavior for "advanced" users to take anything some arbitrary tool -even one that seems official-ish the way we kind of do- at face value.

The paradox of ban-types + {} is that the more TypeScript-savvy the user, the less useful the ban is for them. If a user is writing code that leans heavily into mapped types then they should probably get rid of the ban for that project / area!

[JoshuaKGoldberg]

#8977 was merged into the v8 branch. Yay! 🙌