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

Less verbose class initialization #57367

Closed
6 tasks done
mmkal opened this issue Feb 10, 2024 · 15 comments
Closed
6 tasks done

Less verbose class initialization #57367

mmkal opened this issue Feb 10, 2024 · 15 comments
Labels
Duplicate An existing issue was already created

Comments

@mmkal
Copy link

mmkal commented Feb 10, 2024

πŸ” Search Terms

class verbose, class boilerplate

βœ… Viability Checklist

⭐ Suggestion

Initializing classes which take an options-bag constructor parameter, and copy properties of the options-bag into fields, is very boilerplatey. It'd be good if a) a class could say it implements an interface without having to explicitly redeclare all the fields and b) if it could use something like Object.assign(this, options) to set many fields in one go.

πŸ“ƒ Motivating Example

Here's an example of a Column class, say representing a database table column:

interface ColumnParams {
  table: string
  name: string
  type: string
  nullable: boolean
}
class Column implements ColumnParams {
  table: string
  name: string
  type: string
  nullable: boolean

  constructor(public params: ColumnParams) {
    this.table = params.name
    this.name = params.table
    this.nullable = params.nullable
  }

  get quotedName() {
    return quoteIdentifier(this.name)
  }

  // ...other methods
}

Note that each field in ColumnParams results in three lines of code, only one of each carries any real information/intent in terms of what the program is supposed to do. And in fact the constructor has two bugs in it:

  • we're doing this.table = params.name and this.name = params.table. This is a mistake that's easy to write and very hard to catch - the compiler won't catch it.
  • we also forgot to set this.type = params.type. If the tsconfig has strictNullChecks: true this will be caught, but plenty of projects don't have that (this example is from a port of a python library, which I hope to one day enable strictNullChecks on, but that will require diverging from the port, so won't happen immediately)

Plus, every time we add a property to ColumnParams (say we add column: string), we risk regressing these bugs after fixing.

It'd be great if we could do this:

interface ColumnParams {
  table: string
  name: string
  type: string
  nullable: boolean
}
class Column implements ColumnParams {
  constructor(public params: ColumnParams) {
    Object.assign(this, params)
  }

  get quotedName() {
    return quoteIdentifier(this.name)
  }

  // ...other methods
}

That way:

  1. There's less code - the signal-to-noise ratio is higher. There's only one place the properties + types are declared
  2. No possibility of mixing up properties
  3. No possibility of forgetting to assign properties
  4. Maintainability is higher - adding a property is a one-liner

There are cases where the above might not be wanted - say there are some properties of ColumnParams that shouldn't be assigned to this - but of course the old form would continue to work.

πŸ’» Use Cases

  1. What do you want to use this for? Writing classes with less error-prone boilerplate
  2. What shortcomings exist with current approaches? It's verbose and error prone, see bugs in example above
  3. What workarounds are you using in the meantime? Using plain functions instead of classes can allow relying more on type inference, but classes have benefits and fit plenty of use-cases better
@MartinJohns
Copy link
Contributor

Duplicate of #26792 / #28883.

@mmkal
Copy link
Author

mmkal commented Feb 11, 2024

@MartinJohns thank you! Those cover Object.assign(this, params), do you know if something exists for allowing skipping the redeclaring of interface fields on a class?

@fatcerberus
Copy link

fatcerberus commented Feb 11, 2024

do you know if something exists for allowing skipping the redeclaring of interface fields on a class?

That sounds like either #36165 or #32082. It's unlikely you'd be able to omit the fields entirely though - otherwise if you wrote

class A implements B {}

then TS would have to include the fields from B inside A in the output at compile-time, which falls under the category of type-directed emit and would surely be rejected for that reason (TS as a rule only erases types, and a class without explicitly declared fields behaves differently at runtime than one with them).

@mmkal
Copy link
Author

mmkal commented Feb 11, 2024

do you know if something exists for allowing skipping the redeclaring of interface fields on a class?

That sounds like either #36165 or #32082. It's unlikely you'd be able to omit the fields entirely though - otherwise if you wrote

class A implements B {}

then TS would have to include the fields from B inside A in the output at compile-time, which falls under the category of type-directed emit and would surely be rejected for that reason (TS as a rule only erases types, and a class without explicitly declared fields behaves differently at runtime than one with them).

#32082 looks close, thank you. Re type-directed emit, I wouldn't think so.

I'm suggesting that these become
equivalent:

interface B {
  name: string
}

class A implements B {
}

And:

interface B {
  name: string
}

class A implements B {
  name: string
}

i.e. ok if strict null checks are off, but gets "name is not definitely assigned in the constructor" with strict null checks. That's where Object.assign complements it well:

interface B {
  name: string
}

class A implements B {
  constructor(params: B) {
    Object.assign(this, params)
  }
}

This would make the error go away but js output is unaffected.

So maybe it's still worth having one issue for those two changes together even if each is somewhat covered by those linked duplicates.

@fatcerberus
Copy link

I'm suggesting that these become equivalent

Yes, that's type-directed emit, because TS would have to emit the name field in the output for A for them to behave the same at runtime. class A { foo; } in JS is not the same as class A {}.

@mmkal
Copy link
Author

mmkal commented Feb 11, 2024

I'm suggesting that these become equivalent

Yes, that's type-directed emit, because TS would have to emit the name field in the output for A for them to behave the same at runtime. class A { foo; } in JS is not the same as class A {}.

Hmm is that right? This playground shows no effect of a field that's never set.

Typescript:

class A {
  name: string
}

Output:

class A {
}

@MartinJohns
Copy link
Contributor

The difference appears when you enable useDefineForClassFields, which is enabled by default for ES2022 and later.

@mmkal
Copy link
Author

mmkal commented Feb 11, 2024

Interesting, I had missed that, thanks for explaining. I agree that this is a non-starter if it allows interfaces to affect emit. But couldn't this still be pretty easily workable:

interface B {
  x: string
}
class A implements B {
}

Emits

class A {
}

Or more realistically, since the above class is fairly pointless:

interface B {
  x: string
}
class A implements B {
  constructor(params: B) {
    Object.assign(this, params)
  }
}

Emits:

class A {
  constructor(params) {
    Object.assign(this, params)
  }
}

This is already what typescript emits. The only thing new would be that this second example no longer has a compiler error (Class 'A' incorrectly implements interface 'B'. Property 'x' is missing in type 'A' but required in type 'B'). Playground

Could that even be a useful change not just for the sake of verbosity - what if you have a specific reason to want to avoid the Object.defineProperty(...) stuff emitted by declaring the field explicitly?

@RyanCavanaugh RyanCavanaugh added the Duplicate An existing issue was already created label Feb 12, 2024
@mmkal
Copy link
Author

mmkal commented Feb 12, 2024

A quite silly workaround, but that does seem to work ok at compile time and runtime:

export function AutoThisAssigner<T>(): new (params: T) => T
export function AutoThisAssigner<T, X extends new (...args: any[]) => any>(
  C: X,
): new (params: T, ...args: ConstructorParameters<X>) => T & InstanceType<X>
export function AutoThisAssigner<T, X extends abstract new (...args: any[]) => any>(
  C: X,
): new (params: T, ...args: ConstructorParameters<X>) => T & InstanceType<X>
export function AutoThisAssigner(C: any = Object) {
  return class extends C {
    constructor(params: any, ...args: any[]) {
      super(...args)
      Object.assign(this, params)
    }
  }
}

Can be used like this (note, no explicit constructor, no duplicating field types, no line-by-line field setting, but you still get type safety):

interface ColumnParams {
  table: string
  name: string
  type: string
  nullable: boolean
}
class Column extends AutoThisAssigner<ColumnParams>() {
  quotedName() {
    return JSON.stringify(this.name)
  }
}

const column = new Column({
  table: 'foo',
  name: 'bar',
  type: 'text',
  nullable: false,
})

console.log(column.table, column.quotedName())

It works by abusing class extension to be a cross between extends and implements, so it's less than ideal. You can also extend another class with the overloaded version:

interface GeneratedColumnParams {
  expression: string
  stored: boolean
}

class GeneratedColumn extends AutoThisAssigner<GeneratedColumnParams, typeof Column>(Column) {
  validate() {
    if (!this.expression.includes('foo bar')) {
      throw new Error(`Invalid expression: ${this.expression}`)
    }
  }
}

const generatedColumn = new GeneratedColumn(
  {
    expression: 'foo bar',
    stored: true,
  },
  {
    table: 'foo',
    name: 'bar',
    type: 'text',
    nullable: false,
  },
)

generatedColumn.validate()
console.log(generatedColumn.table, generatedColumn.quotedName(), `is stored: ${generatedColumn.stored}`)

Playground for anyone interested.

@MartinJohns @RyanCavanaugh since this is marked as a duplicate, but the issues it duplicates are years old, would you recommend something like this if we want something like this functionality now? Or is there an existing community solution that does something like this? Or are there hidden pitfalls to the above do you think?

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Feb 13, 2024

@mmkal runtime functionality is explicitly out-of-scope. Your solution looks good to me

@mmkal
Copy link
Author

mmkal commented Feb 13, 2024

@mmkal runtime functionality is explicitly out-of-scope. Your solution looks good to me

@RyanCavanaugh I'm not proposing runtime functionality. Sorry if it was unclear - I'm proposing no changes to emit whatsoever. Just to make this no longer a compile error (i.e. catch up with js - what's emitted right now runs the way most people would expect, despite the compiler slapping your wrist):

interface A {
  x: 1
  y: 2
}

class B implements A {
  constructor(a: A) {
    Object.assign(this, a)
  }
}

Emit stays the same, so no new runtime functionality is introduced. My previous comment was just a hack that allows you to simulate the convenience of this with current typescript. But it's just a hack, involves dynamically building a class etc. I'm not proposing that hack be incorporated into typescript.

@RyanCavanaugh
Copy link
Member

@mmkal is the feature proposal that Object.assign get special treatment, or that saying B implements A just implies that the fields must be there?

@mmkal
Copy link
Author

mmkal commented Feb 13, 2024

Both. I think either one is useful, and perhaps covered by other tickets, but the two together allow a significant reduction in noise and likelihood of errors. I thought the use case might be clearer/more convincing when proposed together.

Edit:

saying B implements A just implies that the fields must be there

I'd phrase it as B implements A implies that the fields must be set in the constructor (by the developer, whether it bethis.foo = ... or Object.assign(this, ...)). Not that they magically get generated or anything.

@snarbies
Copy link

So when you implement an interface but are missing properties from the interface in your class declaration, would they be implicitly declared (but still require assignment)?

interface ExampleLike {
    interfaceMember: number;
}

class Example implements ExampleLike {
    // Not needed, implicit via `implements` clause
    //interfaceMember: number;

    public constructor(init: ExampleLike){
        // Satisfies assignment requirement
        Object.assign(this, init);
    }
}

In this case I would expect Example.interfaceMember to behave like an ad-hoc property (not part of class declaration in emit).

@typescript-bot
Copy link
Collaborator

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

@typescript-bot typescript-bot closed this as not planned Won't fix, can't repro, duplicate, stale Feb 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

6 participants