Check 2612 should be made optional

[nomagick]

Suggestion

🔍 Search Terms

useDefineForClassFields
declare override
'override' modifier cannot be used with 'declare' modifier.(1243)
Property 'resident' will overwrite the base property in 'AnimalHouse'. If this is intentional, add an initializer. Otherwise, add a 'declare' modifier or remove the redundant declaration.(2612)

✅ Viability Checklist

My suggestion meets these guidelines:

• This wouldn't be a breaking change in existing TypeScript/JavaScript code
• This wouldn't change the runtime behavior of existing JavaScript code
• This could be implemented without emitting different JS based on the types of the expressions
• This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
• This feature would agree with the rest of TypeScript's Design Goals.

⭐ Suggestion

Check 2612 should be made optional.

Property_0_will_overwrite_the_base_property_in_1_If_this_is_intentional_add_an_initializer_Otherwise_add_a_declare_modifier_or_remove_the_redundant_declaration
https://github.com/microsoft/TypeScript/blob/343191217575c20e2ee7845927d5ca23981cc663/src/compiler/checker.ts#L40954-L40972

📃 Motivating Example

Consider this snippet:

class DogHouse extends AnimalHouse {
  @Prop({
    default: {
      animalStuff: 'food',
      dogStuff: 'dog food',
    }
  })
  override resident!: Dog;
}

This is valid with useDefineForClassFields set to false.

When useDefineForClassFields is set to true, which is the new default of es2022 and later;
the following error is thrown: Property 'resident' will overwrite the base property in 'AnimalHouse'. If this is intentional, add an initializer. Otherwise, add a 'declare' modifier or remove the redundant declaration.(2612)

This error seems reasonable at first, but it doesn't make sense in the long run.
It only makes sense in the transition period before new es class field behavior becomes universal.

Suppose after 10 years, In year 2032, it's know to all that a class field override will redefine the property in the constructor.

class DogHouse extends AnimalHouse {
  @Prop({
    default: {
      animalStuff: 'food',
      dogStuff: 'dog food',
    }
  })
  override resident!: Dog;
}

Why this snippet triggers error? It's completely valid.
It means the property will be overridden, and the ! indicates that the property is not assigned inside the constructor, but to be assigned by some other means later in runtime. And @Prop is probably doing all the magic behind the scene.

Just pass this through to javascript:

class DogHouse extends AnimalHouse {
  @Prop(...) // To be specified by ecma decorator
  resident;
}

Perfect! We are done!

Ok. That's for all the story.

I expect such option (to disable error 2612) be implemented sooner or later, and eventually becomes default, just like useDefineForClassFields.

I would like to have this option sooner.
It's breaking my valid es2022 compatible code.
Ironically, it's preventing me from adopting the es class field new behavior now, even if my code is capable of the change.

I can PR this if you prefer.

💻 Use Cases

In real-world scenarios, class properties are heavily used with inherence and overrideing.
Such snippet is considered valid and clear:

class DogHouse extends AnimalHouseAfter10TimeExtendingAndHave100InheritedFieldsOverridingEachOther {
  @Prop({
    default: {
      animalStuff: 'food',
      dogStuff: 'dog food',
    }
  })
  override resident!: Dog;
}

Especially when you already have embraced the new mindset of class field being [[define]]ed.
But not so clear with declare coming out of nowhere, even preventing override being specified.

So make this check optional. And disable this check by default when es class field becomes universal.

[nomagick]

With proposed compiler option allowImplicitDefineForClassFields.

[fatcerberus]

Since the error message says

If this is intentional, add an initializer

I think I would ultimately agree that writing...

override resident!: Dog;

...should suppress the error (without an additional compiler option), since ! means "treat the property as initialized".

[fatcerberus]

@nomagick Thanks, I understand what you want now. I was confused in the other issue because it seemed like you needed the property to be removed from the output for this to work at runtime, in which case you would definitely want to use declare to make that clear to people reading the code. Since you don't need to elide the property though and this is legal standards-compliant JS, I agree that the ! should be sufficient here.