`override` modifier should able to be used with `declare` modifier

[nomagick]

Suggestion

🔍 Search Terms

useDefineForClassFields override declare 'override' modifier cannot be used with 'declare' modifier. ts(1243) Property 'a' will overwrite the base property in 'Cls'. 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:

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

⭐ Suggestion

When declaring class fields:

The override modifier should be able to be used with declare modifier.

Have ! work in the place of declare OR: Remove this error if ! or override is specifically given: Property 'a' will overwrite the base property in 'Cls'. If this is intentional, add an initializer. Otherwise, add a 'declare' modifier or remove the redundant declaration.(2612)

📃 Motivating Example

Given a decorator Prop() that somehow declares some metadata on the class field, that would later be used in some other technical components like Database ORMs or Validators, etc.

Consider this example, before useDefineForClassFields set to true:

interface Animal {
  animalStuff: any;
}
interface Dog extends Animal {
  dogStuff: any;
}

class AnimalHouse {
  internalProp: number;

  @Prop({
    default: {
      animalStuff: 'food',
    }
  })
  resident: Animal;

  @Prop({
    default: 'foo'
  })
  someOtherProp: string;

  constructor(animal: Animal) {
    this.resident = animal;
  }
}

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

Note that the DogHouse is defined quite straightforward and expected in a general sense.

Now after useDefineForClassFields set to true, the current way of specifying the DogHouse becomes:

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

It doesn't look too bad, but things can get more complicated:

class DogHouse extends AnimalHouse {
  override internalProp: 42;

  @Prop({
    default: {
      animalStuff: 'food',
      dogStuff: 'dog food',
    }
  })
  declare resident: Dog;

  @Prop({
    default: 'foo'
  })
  override someOtherProp: string = 'foo';

  @Prop({
    default: 'foo'
  })
  someOtherProp2: string;
}

Now it sucks.

The consistency and readability from using override is completely screwed up by declare.

At least the override should be allowed to be used with declare to make it suck a little less:

class DogHouse extends AnimalHouse {
  override internalProp: 42;

  @Prop({
    default: {
      animalStuff: 'food',
      dogStuff: 'dog food',
    }
  })
  declare override resident: Dog;

  @Prop({
    default: 'foo'
  })
  override someOtherProp: string = 'foo';

  @Prop({
    default: 'foo'
  })
  someOtherProp2: string;
}

The best case however, is to have ! work in the place of declare:

class DogHouse extends AnimalHouse {
  override internalProp: 42;

  @Prop({
    default: {
      animalStuff: 'food',
      dogStuff: 'dog food',
    }
  })
  override resident!: Dog; // No declaration emit

  @Prop({
    default: 'bar'
  })
  override someOtherProp: string; // With declaration emit

  @Prop({
    default: 'foo'
  })
  someOtherProp2: string;
}

💻 Use Cases

As example suggested.

The code readability degradation from declare is unacceptable.

I am using typescript with useDefineForClassFields set to off now.

Please consider my suggestion..

[fatcerberus]

The best case however, is to have ! work in the place of declare

! on a property already means something else though, specifically "don't check that this property is assigned in the constructor" so that would be a rather large breaking change.

[nomagick]

"don't check that this property is assigned in the constructor"

I'm a little disagree with this interpretation. Isn't that mean the same "Don't check/do-anything about the initiation of this property ."

Even if you disagree with !, !! is still better than declare.

Given what useDefineForClassFields does, it is the one that's making the breaking changes.

[fatcerberus]

Isn't that mean the same "Don't check/do-anything about the initiation of this property ."

Yes, that’s what ! on a property already does, because there are patterns where a property does get initialized but TS can’t verify it at compile-time (e.g. if the property is initialized by an indirect method call in the ctor).

If you want something to not be emitted, use declare. That’s why it’s there.

[nomagick]

You see, I already have multiple layered inherited classes, a very complex structure.

Like in the examples, most subclasses don't have a constructor. They are just decorated differently. When subclass overrides parent property, they are declared with override prop!: Type;

I don't really care about the emit/no-emit of the properties. I would like my code continue to work after I specify target: 'es2022' in tsconfig.json, and the code readability and maintainability not drop.

And now, with this useDefineForClassFields defaults to true. Suddenly all my code is fucked up and It needs me to add declare everywhere(damages readability) and remove all the override from deeply inherited complex data classes(damages maintainability).

This doesn't work.

People choose TypeScript for its readability and maintainability. Javascript that Scales, remember?

It's a shame that TypeScript is willing to drop both for useDefineForClassFields nonsense.

[nomagick]

Another approach.

Remove this error if ! / override is specifically given: Property 'a' will overwrite the base property in 'Cls'. If this is intentional, add an initializer. Otherwise, add a 'declare' modifier or remove the redundant declaration.(2612)

I don't know what's happening around useDefineForClassFields and declare override and ! But you guys should rethink it.

I don't think you will get a good adoption of useDefineForClassFields if you keep the way it is now.

Maybe Interview some real-world users.

[fatcerberus]

If your issue is useDefineForClassFields breaking your code, you can just set it to false. If that's not possible because you depend on third-party code that expects the [[Define]] behavior, well, you need to take that up with the ECMAScript committee, as that is how the class fields spec is written and the backward-compatibility mechanism TS has provided is to manually set useDefineForClassFields: false. If you were writing plain JS with class fields, you wouldn't even have that luxury.

Also, please tone down the language and hostility if you don't want the TS devs to lock your issue.

[nomagick]

@fatcerberus Apologies If my words are too offensive. I'm not very good at English after all. I just wanted to express my feeling on this issue and the direction it is developing. Now I believe you have acknowledged my frustration. I hope the user frustration still means something to TS devs. Maybe I'll start a new issue and rephrase all about this after further discussion.

It's not about the new ES Class define behaviour, not on their part. This is exactly a TypeScript issue.

According to your own reply here, you are well aware of no matter what TS do, the eventual behaviour will change because of the underlying runtime. Then it's ok to treat it transparently and not throw errors on this:

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

At least it should have an option to disable this error. 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)

FYI the decorator is working as expected. I authored those decorators. It's using the metadata registering approach. It's not about the code not working; it works. It's about TypeScript not allowing this formerly-valid clear, precise way of writing code anymore, and proposes another much less favourable approach, known as declare.

[nomagick]

I've put it in another way, hopefully it can convenience you more. https://github.com/microsoft/TypeScript/issues/51536

[termi]

Please consider this case.

import type { Model, ModelView, Prop } from 'myORMLib';

@Model
class SomeModel {
    @Prop({ type: 'uint16' })
    prop1!: number;
    // ...   propN: type;
    @Prop({ maxLength: 256 })
    prop15!: string;
}

// make all properties readonly
// So ugly, but works! (ignore the 2612 error here)
@ModelView
class ReadonlySomeModel extends SomeModel {
    @Prop({ readonly: true })
    readonly prop1!: SomeModel["prop1"];
    // ...   propN: SomeModel["propN"];
    @Prop({ readonly: true })
    readonly prop15!: SomeModel["prop15"];
}

I have to write propN: SomeModel["propN"] for every property i want to decorate in ReadonlySomeModel subclass.

And now imagine how cool it would be if decorators works with declare keyword!?

// make all properties readonly
// 🦄🌈 beauty and shine 🌈​🦄​​ 
@ModelView
class ReadonlySomeModel extends SomeModel {
    @Prop({ readonly: true })
    declare readonly prop1;
    // ...   declare readonly propN;
    @Prop({ readonly: true })
    declare readonly prop15;
}