Type rules are not followed for parent classes with generic types and child class fields

[RobertSandiford]

Bug Report

Child class, fields, properties, generic class inheritance, type rules, additional properties in objects

Child class fields do not follow type rules

🕗 Version & Regression Information

Seen in 4.9.5 + 5.1.6. Not aware of a version without it.

• This is the behavior in every version I tried, and I reviewed the FAQ for entries about anything relevant

⏯ Playground Link

Playground

💻 Code

class P<T> {
 t?: T
}

type U = {
 a: number
 b: number
}

class C1 extends P<U> {
 t = { // expected error, b is missing
 a: 1,
 }
}

const u: U = {
 a: 1,
 b: 2,
 c: 3 // expected error, c is additional
}

class C2 extends P<U> {
 t = {
 a: 1,
 b: 2,
 c: 3 // unexpectededly allowed
 }
}

🙁 Actual behavior

The (object with 1 property) assigned to C1.t does not match the type U. As expected.
The (object with 3 properties) assigned to u does not match the type U. As expected.
However the (object with 3 properties) can be assigned to C2.t without an error. Not expected.

🙂 Expected behavior

Assignments to P.t (t is type U) to behave in the same was as assignments to u (type U), type error thrown at line 26

[fatcerberus]

Sigh… the excess property check is not an actual type rule and shouldn’t be relied on to enforce runtime invariants. It is not an implementation of exact types (#12936).

[MartinJohns]

Additionally to what Bruce Pascoe (@fatcerberus) wrote, your property C1.t lacks a type annotation, so the type of the property is inferred from it's assignment. It is not using the type from the base class. This is tracked by #10570.

[RobertSandiford]

I put together this playground to show the root of the issue (thanks for the advice) - the way that excess property checks and indexing checks are applied.

Playground

type T = {
 a: number
}

const t: T = {
 a: 1,
 b: 2 // Excess Property Check error
}

const u = {
 a: 1,
 b: 2
}

const v: T = u // No Error! - Excess Properties are allowed

v.c = 3 // Excess Property Check kicks in again

const k = 'd'
v[k] = 4 // Expression of type 'd' cannot be used to index v. Effectively Excess Property Check is back

I'm not a fan of the behaviour. It seems quite arbitrary, and is likely to mislead new developers into think that TypeScript is guaranteeing that T will always be restricted to known properties.

I would much rather see TS stick to one route or another. I would prefer something like Exact object types and loose objects types per #12936

[RyanCavanaugh]

I'm not a fan of the behaviour.

Detecting misspelled properties in object literals was basically the highest-requested behavior change in the entire language at the time we did it.

[RobertSandiford]

I'm not a fan of the behaviour.

Detecting misspelled properties in object literals was basically the highest-requested behavior change in the entire language at the time we did it.

Yes I can see the desire for it. The issues right now are consistency (the example in this ticket), and understandability (that its a sanity check and not a type violation).

How about this approach:

1. Excess Property Checks and Key checks are marked as warnings, not errors (I don't know if TS has any concept of this currently), and marked with yellow squiglies in code editors (where possible).

2. Express Property Check wording is changed to something like warning: property 'c' in type 'U' is not recognised. Is this a mistake?

3. Excess Property Check applies when initializing an untyped property in a derived class, that is implementing an abstract typed property (the example at the top of this ticket). This gets into issue Inherited typing for class property initializers #10570 - I'm not going to fully consider that now.

4. Adding a new property to an object updates the type of that object instead of setting it to any. Example:

const o = {
 a: 1
}

o[b] = 2 // Element implicitly has an 'any' type because expression of type 'any' can't be used to index type '{ a: number; }'

Here the type of o would be updated to { a: number, b: number }

The top of the object referred to be o would not be universally updated.

const o = {
 a: 1
}

function f(oo: typeof o) {
 oo[b]
}

o // { a: 1 }

But this does not violate anything on the basis that TS objects are truly extensible. Developers can easily assign the result of f if they want to keep the new typings.

5. The wording of the new property warning would be set to something like warning: property 'c' in type 'U' is not recognised. Is this a mistake? or warning: property/indexer string in type 'U' is not recognised. Is this a mistake?

6. Allow these "warnings" to be disabled via a default entry in the tsconfig (EPC can be disabled, but the option is not listed by default, and apparently is deprecated, to be removed in the future.

7. Ideally Exact types would be brought in to provide bullet proof restrictions where required.

Advantages of this are

A) its easier for TS users to understand the TS objects are extensible, and cannot be assumed not to have additional properties. Will avoid some bugs and mistakes, albeit rare ones.

B) Developers who prefer to work with objects in the JS sense, that is freely extensible, can do so more conveniently, be it ignoring the yellows or disabling the checks. Type info is now present on extended objects automatically, without needing explicit type definitions and conversions. (note: the spread operator can be used to achieve this fairly easily at current, but that could become laborious with more complex algos)

C) Checks are added to prop initialization in derived classes (depending on #10570)

Disadvantages: I don't see any, besides the work involved.

Back Compatibility issues:

i) Derived class prop initialization checks would lead to warnings in existing code. If these "warnings" never prevent code emit, this would not be problematic.

ii) Typing changes to objects that have new properties assigned could lead to type errors. Variables that were any will not be typed. However since any type errors created this way are likely to indicate a bug in the program, this seems acceptable.

--

The opposite approach is simply to double down, saying that all object types are, in essence, Exact, and cannot have extra properties in any situation. But I think this would break so much, given the object types are currently not restrictive, to make it borderline unworkable.

I look it from the point of view that JS objects are extensible, and trying to redefine the role of objects in TS is overly confrontational. Already today handling of objects is a bit of a pain point in converting JS code to TS, it doesn't feel like the right approach to take, vs something like opt in "Exact" types.

--

I'm open to contributing code by the way.

[RobertSandiford]

Thinking about whether the idea that TS objects are extensible is correct.

Playground

Is this behaviour intentional?

let o = {
 a: 1
}

function extend(o: { [key: string]: {} }, key: string, value: {}) { // no errors
 o[key] = value
}

extend(o, 'b', 2)

console.log(o)
// o = { a: 1, b: 2 }
// o's type = { a: number }

I don't see how the type { a: number } is satisfying the constraint { [key: string]: {} }. ({} is being used as "any value except null and undefined)

Another experiment Playground 2

[RyanCavanaugh]

You're describing an extremely large set of changes to address a fairly narrow surprise here. There's not going to be a lot of appetite for throwing out this many babies with the bathwater for the sake of addressing this.

I agree the error message wording could be improved. Open to useful suggestions on that one.

OT: It seems like you're new to TS and are logging bugs about every surprise you encounter along your learning journey -- keep in mind that the behavior you see is largely designed to mitigate previous users' surprises that they encountered when learning TS that had other behavior. It's not practical to reduce this amount to zero; everyone brings their own different preconceptions around how a language should work. Have you read the Handbook? It's designed to address some of these questions.

[RobertSandiford]

You're describing an extremely large set of changes to address a fairly narrow surprise here. There's not going to be a lot of appetite for throwing out this many babies with the bathwater for the sake of addressing this.

I agree the error message wording could be improved. Open to useful suggestions on that one.

OT: It seems like you're new to TS and are logging bugs about every surprise you encounter along your learning journey -- keep in mind that the behavior you see is largely designed to mitigate previous users' surprises that they encountered when learning TS that had other behavior. It's not practical to reduce this amount to zero; everyone brings their own different preconceptions around how a language should work. Have you read the Handbook? It's designed to address some of these questions.

I read probably some of the handbook when I started out with TS 2-3 years ago. There is quite an inconsistency in how TS handles object typing. On the way hand we have the "interface" concept a.k.a. structural a.k.a. duck typing, where an object only needs to match the expressed properties, and doesn't need to match exactly. There there are the excess property checks and indexing checks, which it sounds like were added in later based on user demand. But these 2 concepts are at odds, and the way the TS sometimes follows one and sometimes the other is quite confusing, especially so given the messages involved. This is why the key part of my suggestion about was to change the display of EPC/Indexing errors to being clearly warnings, so as to establish a clear heircarchy: object typings are structural typings but here are some warnings, because maybe you just made a mistake here. I read about structural types when I started - but obviously over time, being hands on, I just became convince that we were dealing with exact types. That's what the messaging in-editor suggests, I started designing my code that way, and haven't yet had a real need to deviate from that. I just find the way that these 2 parts of TS are so at odds with each other quite strange.

I don't think the suggestions above are really throwing anything out - they work with and enhance the current approach. But there are a few minor breaks involved - are they the parts that you refer to as throwing things out, or something else? Is it simply that case that you are trying to avoid all breaks unless really necessary?

I will take the time to read the handbook in detail, when I can.

[fatcerberus]

This is why the key part of my suggestion about was to change the display of EPC/Indexing errors to being clearly warnings, so as to establish a clear heircarchy: object typings are structural typings but here are some warnings, because maybe you just made a mistake here.

TS unfortunately makes no distinction between warnings and errors but check out #55152.

I read about structural types when I started - but obviously over time, being hands on, I just became convince that we were dealing with exact types.

I don't think you're alone. Issues similar to the OP seem to show up about once a week on average. It's been said by the maintainers that the demand for #12936 dropped off sharply after EPC was introduced, but my suspicion is that a lot of these people are misapprehending what the feature actually does (in particular, that it's not intended to enforce runtime invariants) and would still want exact types if they knew the EPC wasn't a full type-level check.

[RobertSandiford]

TS unfortunately makes no distinction between warnings and errors but check out #55152.

Just changing wording will be adequate. Slap a big "Warning:" at the beginning and/or use "are you sure" kind of language.

It's been managed with the typecasts Conversion of type '{ foo: string; }' to type '{ id: 123; }' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.

Doesn't tell you it can't be done, and even tells you how to do it.

Warning: Property 'c' does not exist in type 'U'. Is this a mistake? To continue the assignment, cast the new object to 'U'.

e.g. const user: User = { name: 'Jim', age: 37 } as User

Yellow squiglies are nice, but not the biggest issue.

[fatcerberus]

RobertSandiford You should post your suggestion(s) for wording in #55152 while it’s still open.

[typescript-bot]

This issue has been marked as "Working as Intended" and has seen no recent activity. It has been automatically closed for house-keeping purposes.