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

Rethinking relationships between {} type, object, and primitives #60582

Open
kirkwaiblinger opened this issue Nov 25, 2024 · 15 comments
Open

Rethinking relationships between {} type, object, and primitives #60582

kirkwaiblinger opened this issue Nov 25, 2024 · 15 comments
Labels
Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. Suggestion An idea for TypeScript

Comments

@kirkwaiblinger
Copy link

πŸ”Ž Search Terms

empty object type, {}, type safety violation, unsound

πŸ•— Version & Regression Information

⏯ Playground Link

https://www.typescriptlang.org/play/?ts=5.8.0-dev.20241124#code/GYVwdgxgLglg9mABFAhgawKYGcDyAjAKw2gAoAPALkTkOKgEpEBvAKEXcQHpPEAVACxhZEAJxgBzflAA2AT0RDEKPgGVEGESLgiAdGw4xgicogC85xAAZGrDncQQEWONIw6NWkSQDkAxXhEUSH5ELH44EGkAE0Q8DERwEQwUCH4UPFdvegBufXYAXxZClkcwLChqKiZ8sytclm5EACEQCqhBYSEwbygAGiUwGLCI6Ni3FlRMXFpSOBygA

πŸ’» Code

function takesObject(x: object) {
    // This rightly is a TS error.
    if (x === 0) {
        console.error('This branch should be unreachable');
    }
}

const o: {} = 0;

// But this isn't, and should be.
takesObject(o);

πŸ™ Actual behavior

No error on takesObject(o). {} can be assigned to type object, despite {} meaning "all nonnullish values", whereas object means only JS object types.

πŸ™‚ Expected behavior

Error on takesObject(o). {} is a wider type than object.

Additional information about the issue

This stems from a typescript-eslint investigation into making no-unnecessary-condition be more correct around possibly-falsy "object types", such as {}, or even { toFixed(): string} (to which number may be assigned). See typescript-eslint/typescript-eslint#10378. This also relates to previous (controversial) conversations about whether to flag the {} type with the linter, see, e.g. typescript-eslint/typescript-eslint#8700.

I'm wondering if this was simply an oversight in #49119, which aimed to fix soundness holes with {}?

@jcalz
Copy link
Contributor

jcalz commented Nov 25, 2024

duplicate #56205

@JoshuaKGoldberg
Copy link
Contributor

JoshuaKGoldberg commented Nov 25, 2024

Even if it's a duplicate, given #56205 (comment):

Allowing {} to be used as object is an intentional compatibility hole since object didn't always exist, so people used {} as the next-best thing.

I read this new issue as a request to not do that. As in, now that TypeScript has had better handling around {} and object for a while (refs: #49119, #57735 (comment)). Over in typescript-eslint we've overhauled our lint rules around the empty object types to respect the new behaviors and best practices (typescript-eslint/typescript-eslint#8700 -> https://typescript-eslint.io/rules/no-empty-object-type). This hole is a sticking point. +1 from me on filling in the hole.

@jcalz
Copy link
Contributor

jcalz commented Nov 25, 2024

Is it just {}? So would {a: string} still be assignable to object or not? What about {length: number}?

@JoshuaKGoldberg
Copy link
Contributor

Personally, I'm of a "strict" interpretation: that any { ... } type coincidentally assignable to a non-object primitive should not be assignable to object. Only with object & should they be. Quick examples:

  • {}: as discussed
  • { length: number }: because of string
  • { toPrecision(): string } and { toPrecision(precision?: number): string }: because of number

This is probably tricky from an implementation perspective. But it's right.

@kirkwaiblinger kirkwaiblinger changed the title {} should not be assignable to object {} type should not be assignable to object Nov 25, 2024
@fatcerberus
Copy link

@JoshuaKGoldberg So you're suggesting that this shouldn't be allowed:

let obj: { length: number } = { length: 10 };
const o: object = obj;

Simply because obj could also hypothetically be a string?

@kirkwaiblinger
Copy link
Author

kirkwaiblinger commented Nov 25, 2024

So you're suggesting that this shouldn't be allowed:

let obj: { length: number } = { length: 10 };
const o: object = obj;
Simply because obj could also hypothetically be a string?

If we're unpacking this in detail, negative - your example should be allowed πŸ™‚ But the reason why is control flow narrowing, which TS already does in other cases as well.

This should be disallowed, though.

let obj: { length: number } = { length: 10 };
// ... lots of code
if (Math.random() > 0.5) {
  obj = "not an object!"
}
// ... lots of code
const o: object = obj;

This should also be disallowed.

declare const objOrString: { length: number };
const o: object = objOrString;

Note that similar DevX tricks are applied around nullishness, see, for example,

const constNullishNumber: null | number = 42;
const num1: number = constNullishNumber; // allowed!

let neverReassigned: null | number = 42;
const num2: number = neverReassigned; // allowed!

// vs

let reassigned: null | number = 42;
if (Math.random() > 0.5) { 
  reassigned = null;
}
const num3: number = reassigned; // not allowed!

@fatcerberus
Copy link

fatcerberus commented Nov 25, 2024

@kirkwaiblinger

If we're unpacking this in detail, negative - your example should be allowed πŸ™‚

Yes, that was the point of the comment - it was a counterexample for @JoshuaKGoldberg's original argument.

@kirkwaiblinger
Copy link
Author

kirkwaiblinger commented Nov 25, 2024

@fatcerberus

Yes, that was the point of the comment - it was a counterexample for @JoshuaKGoldberg's original argument.

...and I'm saying I don't see how it's a counterexample at all? It's just an edge case that would need to be handled by the same sleight-of-hand that TS already includes in similar situations.

Regardless, this is perhaps an implementation detail consideration that's not worth spending too many more words going back and forth on before we even find out if this issue will be summarily closed as a duplicate or not πŸ˜†. So I'll leave it there for now to avoid noise in the issue conversation. But definitely an important detail to handle if this does get to the point of implementation! πŸ™‚

@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. labels Nov 26, 2024
@RyanCavanaugh
Copy link
Member

We would need some proposal that creates the desired behavior without unacceptable negative side effects (we've taken several bites at this apple before and not come up with much that was particularly appetizing). No one is going to turn on this behavior if it means every time they write

interface Person {
  name: string
}

they actually need to write

interface _PersonFields {
  name: string;
}
type Person = object & _PersonFields;

to get something that's properly assignable to object, since in practice that's the only kind of thing that can inhabit Person anyway.

IMO the right fix here isn't making { } un-assignable to object, it's making objectness an intrinsic property of writing { } syntax in the first place, and then figuring out how to work backwards to a way to talk about types where you accept a { length: number } and don't care if it's an Array or a string. Maybe that's just giving up and saying that you have to write { length: number } | string.

Or maybe that's some kind of syntax around how the global String type is declared, e.g.

interface String notextends object {
  length: number;
}

which would create a type that didn't have intrinsic object-ness. Point being, you should not (generally) have to do extra work to opt in to the existing behavior as relates to being able to assume that types are object unless known to be primitive.

@kirkwaiblinger
Copy link
Author

kirkwaiblinger commented Nov 28, 2024

@RyanCavanaugh

I am enthusiastically on board with considering how to make it so that { ... } types are objects! I only framed the issue in the way I did on the assumption that that would be a nonstarter. But I agree with every word in #60582 (comment) πŸ™‚

I guess, before thinking too deeply to reinvent the wheel, I would be pretty interested to know how far down the rabbit hole those of you on the TS team have gone when pondering this previously, and what undesirable edge cases might have arisen. Do you have references on this that I might look into?

My first thought is that the overarching model here would be that the runtime JS type, i.e. the result of x === null ? 'null' : typeof x, would additionally be tracked in TS's type system. If for niche usages a Structural<T> intrinsic type needs to be provided, it could return the exact same type but with any associated runtime type (and therefore, for example, Structural<BoxedNumber> would be more or less identical to Structural<number>, apart from possibly readonlyness).

Since I can't help it, I've started to think through some details and consequences of this, but, as mentioned, probably it's best to catch up to where you are first, before dropping walls of text here πŸ˜€

Excited to be having this conversation!

@kirkwaiblinger kirkwaiblinger changed the title {} type should not be assignable to object Rethinking relationships between {} type, object, and primitives Nov 28, 2024
@HansBrende
Copy link

Seems to me that anytime interface is declared, there should be an implicit extends object or & object tacked on.

The typescript built-in primitive types should simply automatically remove that modifier from their corresponding reference interfaces internally (I don't think users themselves need the ability to remove that modifier since we can't create new primitive types! There can only ever be the built-in primitives, right?)

Without an interface declaration however, it seems far more elegant to not assume anything. For example: type X = {} should not extend object automatically, in my opinion: it makes the inheritance hierarchy non-transitive.

However, I would also echo @kirkwaiblinger's request for references to any discussion/pitfalls/undesirable edge cases that have already been covered on this topic: would love to read more into it.

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Dec 2, 2024

I couldn't find any relevant links (unfortunately this isn't very searchable). The root problem is that the inconsistencies here can't be removed, they can just be moved. We say that this program is legal

function foo(s: string) {
  return s.length;
}

But why is it legal? It's legal because the string type also includes properties from the global String type.

Similarly, we allow you to write this program, for the same reason:

function foo<T extends { length: number }>(x: T) {
  return x.length;
}
// Legal call
foo("hello world");

The proposal here implies breaking this consistency: You can no longer take some block of expressions and talk about them in a higher-order fashion if some of those expressions involve property access on primitives. Accessing the length of a string is now a fundamentally different operation than accessing the length of an array.

This raises further problems, consider something like this

const p = "foo";
type X = (typeof p)["length"];

type GetLength<T> = T extends { length: infer U } : U ? never;
type Y = GetLength<typeof p>;

Is the type X even legal? One argument says yes, because it's what you'd get if you wrote p.length. Another argument says no, because this should be basically equivalent to GetLength, which would now produce never instead.

Every place we produce or ingest a property name, you now have to think about whether that property name exists as part of an object syntax or doesn't. e.g. what's keyof string?

This also implicitly breaks common "branding" patterns like string & { isNormalized: true } since string & object is obviously never.

I'd also just weigh in that making interface implicitly extend object but not type X = { } seems extremely inconsistent and, arguably, counterintuitive? If anything I'd choose the opposite. You can imagine making it legal to write interface Foo extends object { (and making that idiomatic in your codebase) but there's not as much syntactic clarity in having to jam in an & object into a type declaration. But either way it introduces "yet another thing you have to know", which isn't great from the perspective of keeping the language learnable.

@RyanCavanaugh
Copy link
Member

Other issues that became apparent when making an extremely rough prototype (baseline results here)

Today unknown can be freely decomposed to and from the union {} | null | undefined. This would have to instead be the union {} | null | undefined | string | boolean | number | symbol | bigint. Not a dealbreaker but it's another complication to deal with.

Many JSDoc comments use Number instead of number, so that would be breaking almost everywhere. We could reinterpret Number in that context (we already do for other types) but it's another inconsistency.

string would no longer be Iterable<>; arguably this is a good thing because iterating a string is almost never a good idea, but if you really did mean to write a for/of loop on some programmatic data, it's unclear what you would do instead.

I think it'd be super weird to have string not be { [index: number]: string }, but that's a straightforward consequence.

The definition of NonNullable would have to change. Unclear what the downstream implications of that would be or if that would be a dealbreaker since we can't change the lib types based on compiler settings. I'd have to think about it more.

There's some very real inconsistency around inference of destructuring parameters. Let's say you write

function foo({ length }) { }
foo("bar");

This is legal; if you look at the inferred type of foo it's

function foo(arg: { length: any }) : void

so the call would become illegal. IMO that's pretty weird, since if you had written the equivalent downleveling, you'd be fine. Maybe we could iterate through all the primitives and add them to the signature if they'd be compatible and overlappy with the parameter type, but that seems like spooky action at a distance.

@HansBrende
Copy link

HansBrende commented Dec 3, 2024

@RyanCavanaugh unless I'm mistaken, all of the polymorphic inconsistency issues as well as the branding pattern/opaque type-alias issues you mentioned only exist in the world where {} == object. Hence why I suggested that not be the case, but rather the other way around: {} equivalent to boolean | string | number | bigint | symbol | object, with {} & object syntax available when a reference type is actually required (in most cases though, it is probably not required... the only place in (for example) the standard library I can even think of where object is a required input is Object.create and Object.setPrototypeOf!)

So we come to the problem where the vast majority of functions do not need to assume object in their parameter types (i.e., types in contravariant positions), while the vast majority of custom datatypes are in fact inhabited by object instances.

I suggested interface implicitly extend object simply to make life easier in the latter regard, similar to class declarations, which of course should definitely extends object implicitly (it may be anecdotal, but usually when I see interface declarations, they are built with a non-primitive type in mind, similar to a class). This suggestion was simply meant to target the only pain-point mentioned. However, if that would pose too much of an inconsistency, then allowing interface extends object syntax would absolutely be a viable alternative in my mind.

You also noted that { ... } & object wouldn't have much syntactic clarity. That sentiment is confusing to me since the intersection operator seems to be so idiomatic. We have {} & T to narrow to non-nullable types, so I'm not sure why narrowing to reference types wouldn't be in keeping with the existing landscape. However, one could certainly imagine an alternative syntax such as:

type X = object { ... }

if that would make things clearer! Again though, the number of places where a type signature will actually require object as an input to a function I would think is minimal (I'm anecdotally looking at the standard lib). So I don't think people would need to care about tacking on object very often. (But correct me if I'm wrong.) And doing it the other way would risk just as much, if not more, extra boilerplate, given that many types in contravariant positions would want to opt out of this object requirement.

@HansBrende
Copy link

Another possibility I'd like to throw out to reduce possible boilerplate issues (keeping in mind, as stated above, this is not necessary for the proposal to work--I'm just targeting the only pain-point mentioned): interface extends object is valid syntax, and interface extends object is the default extends clause, but only if there is no explicit extends clause. Users can opt out of this by explicitly stating interface extends unknown, or interface extends XYZ to inherit the behavior of interface XYZ.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

6 participants