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

Combining destructuring with parameter properties #5326

Open
buzinas opened this issue Oct 19, 2015 · 101 comments
Open

Combining destructuring with parameter properties #5326

buzinas opened this issue Oct 19, 2015 · 101 comments
Labels
Effort: Moderate Requires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual". In Discussion Not yet reached consensus Suggestion An idea for TypeScript

Comments

@buzinas
Copy link

buzinas commented Oct 19, 2015

Today, we can take advantage of parameter properties to reduce the boilerplate, e.g:

class Person {
  constructor(public firstName: string, public lastName: number, public age: number) {

  }
}

Since 1.5, we can also use destructuring, e.g:

class Person {
  firstName: string;
  lastName: string;
  age: number;

  constructor({ firstName, lastName, age } : { firstName: string, lastName: string, age: number }) {
    this.firstName = firstName;
    this.lastName = lastName;
    this.age = age;
  }
}

I've tried in many ways to combine both features, but had no success. So:

  1. Is it possible to combine them nowadays, and if yes, how?
  2. If not, could it be an improvement to a future TypeScript version? E.g:
class Person {
  constructor(public { firstName, lastName, age } : { firstName: string, lastName: string, age: number }) {

  }
}

// the code above would possibly transpile to:
var Person = (function () {
    function Person(_a) {
        var firstName = _a.firstName, lastName = _a.lastName, age = _a.age;
        this.firstName = firstName;
        this.lastName = lastName;
        this.age = age;
    }
    return Person;
})();
@xLama
Copy link

xLama commented Oct 20, 2015

I like this.

@DanielRosenwasser
Copy link
Member

@buzinas I originally had this working in #1671, but didn't take it in (check out #1541). I think the issue was that it was relatively dense in semantics. I don't necessarily agree, but maybe someone else on the team can weigh in.

@danquirk
Copy link
Member

Note that in those proposals the parameter property modifier applied to the entire binding pattern (simpler) as opposed to the original suggestion here which in theory supports different visibility modifiers per destructured element (not sure that level of specificity would be worth it).

@buzinas
Copy link
Author

buzinas commented Oct 20, 2015

@danquirk For me, if it's easier for you to do the other way, I don't really care. In fact, that was my first try (e.g public {firstName, lastName, age}), and as soon as it didn't work, I tried to use on each property, and it didn't work too.

It would be great if we could support both (since not always we want to create properties for all the parameters, and when we want, it would be simpler to use public/private only once), but if it's easier to support only one approach, it will be great already.

Probably it's something that people will use more and more, since destructuring is awesome.

@mhegazy mhegazy added Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels Oct 21, 2015
@robianmcd
Copy link

just ran into this. I think either approach would satisfy most use cases. Hope to see this in a future version.

@laurelnaiad
Copy link

The first thing I tried was:

  constructor(thisProps: { public myProperty: string, public myOtherProperty: number }) {
    // etc
  }

Something like this would be a nice-to-have.

@jack-guy
Copy link

Definitely +1 this. The alternative tends to be... bulky.

@albertywu
Copy link

👍

3 similar comments
@hrundik
Copy link

hrundik commented Apr 19, 2016

👍

@aliatsis
Copy link

+1

@lekev
Copy link

lekev commented May 13, 2016

+1

@andriilazebnyi
Copy link

+1

2 similar comments
@yjaaidi
Copy link

yjaaidi commented Aug 9, 2016

👍

@TiuSh
Copy link

TiuSh commented Oct 11, 2016

+1

@RyanCavanaugh
Copy link
Member

Please use the GitHub reactions feature rather than standalone upvote comments. Thanks!

@appsforartists
Copy link

In an attempt to be more DRY using named args and strong types (until something like your proposal lands), I tried this:

interface ExampleArgs {
  firstArg: string;
  otherArg: number;
}

export default class Example implements ExampleArgs {
  firstArg;
  otherArg;

  constructor(kwargs:ExampleArgs) {
    return Object.assign(this, kwargs);
  }
}

but got Member 'firstArg' implicitly has an 'any' type. errors for every argument. ☹️

@RyanCavanaugh
Copy link
Member

Write this instead

interface ExampleArgs {
  firstArg: string;
  otherArg: number;
}

export default class Example {
  constructor(kwargs:ExampleArgs) {
    return Object.assign(this, kwargs);
  }
}
export interface Example extends ExampleArgs { }

@appsforartists
Copy link

Thanks. I had to separate the exports from the declarations to make that work:

interface ExampleArgs {
  firstArg: string;
  otherArg: number;
}

class Example {
  constructor(kwargs:ExampleArgs) {
    return Object.assign(this, kwargs);
  }
}

export default Example;
interface Example extends ExampleArgs { }

@markrendle
Copy link

This would be incredibly useful for hydrating class-based models from JSON.

As in

export interface PersonDto {
    name?: string;
}

export class Person {
    constructor(public {name}: PersonDto = {}) {
    }
}

@yjaaidi
Copy link

yjaaidi commented Nov 11, 2016

Meanwhile we get this feature, here's the workaround:

export class PersonSchema {
    firstName: string;
    lastName: string;
    email?: string; // Thanks to TypeScript 2, properties can be optional ;)
}

export class Person extends PersonSchema {

    constructor(args: PersonSchema = {}) {
        Object.assign(this, args);
    }

}

The side effect is that if args has extra fields they will be copied into your new object.

This will also be your copy constructor.

@RyanCavanaugh RyanCavanaugh added Help Wanted You can do this and removed In Discussion Not yet reached consensus labels Nov 15, 2016
@RyanCavanaugh RyanCavanaugh added this to the Community milestone Nov 15, 2016
@RyanCavanaugh
Copy link
Member

Accepting PRs to implement constructor(public {name1, name2}) forms (i.e. no public / private etc inside the {}s).

@appsforartists
Copy link

@RyanCavanaugh, you're proposing that

constructor(public { name1 = value1, name2 = value2 }: { name1: string, name2: string }) {}

would desugar to

constructor({ name1 = value1, name2 = value2 }) {
  this.name1 = name1;
  this.name2 = name2;
}

right?

@dmoebius
Copy link

dmoebius commented Dec 2, 2016

Why is this discussion only about constructors? Wouldn't this apply to other methods as well?!?

@KutnerUri
Copy link

this is great, and I'm looking forward to #44912.

Why only pre-destructure syntax, though?

constructor(public { id, secretHash }: ...) {}
// vs
constructor({ public id, private secretHash }: ...) {}

@gtarsia
Copy link

gtarsia commented Dec 7, 2021

I find this combination of declaration merging and Object.assign to be the easiest way to deal with this at the moment:

interface ClsAttrs {
  name: string;
}

interface Cls extends ClsAttrs {}
class Cls {
  constructor(attrs: ClsAttrs) {
    Object.assign(this, attrs)
  }
}

export default Cls

It's not great either because maybe you want to make certain props optional in the constructor arguments but required as a class' attribute.

For some reason exporting Cls doesn't play well with other exports in the same module, because of that is why I wrote export default Cls at the end.

zechmeister added a commit to digitalservicebund/mitra-frontend that referenced this issue Feb 7, 2022
Unfortunately, ts does not support mixing destructuring and parameter
properties right now
(see microsoft/TypeScript#5326).
This still imporves maintainability.

Addresses MIT-166
@sandersn sandersn added In Discussion Not yet reached consensus and removed Help Wanted You can do this labels Apr 7, 2022
@luke-robertson
Copy link

Still want this in 2022

@larssn
Copy link

larssn commented Jun 1, 2022

I'd love to have a Dart-like syntax for this.

We'd be able to do:

class Test {
  constructor({
    private param1: string,
    protected param2: number,
    public param3?: string // optional
  }); // <-- Might as well make the constructor branckets optional too!
}

const test = new Test({
  param1: 'hello',
  param2: 1337,
  param3: 'world',
});

console.log(test.param3); // -> 'world'

@ianldgs
Copy link

ianldgs commented Jun 1, 2022

@larssn that is what I proposed here:
#5326 (comment)
However, someone rightfully pointed out that it conflicts with existing JS syntax:
#5326 (comment)
As an alternative, this could be added in front to differentiate:
#5326 (comment)

But to be honest, I haven't used classes in JS/TS in a while. And not planning to use it again anytime soon.
To reflect: what is the difference between immutable pojos + functions vs immutable classes?

interface Obj {
  readonly foo: string;
  readonly bar: string;
}

function foobar(obj: Obj) {
  return obj.foo + obj.bar;
}
class Obj {
  public readonly foo: string;
  public readonly bar: string;
  
  // whatever constructor
  
  foobar() {
    return this.foo + this.bar;
  }
}

For me, there isn't much, except that with immutable pojos + functions I don't have to worry about this context.

@larssn
Copy link

larssn commented Jun 1, 2022

@ianldgs There's should be no difference between them, it's just semantics.

Anyway, consider my post to just be a big fat thumbs up for your post then! 🙂
But I don't understand why renaming properties (#5326) would be a hindrance - tooling shouldn't influence the spec, unless I'm misunderstanding something.

@ianldgs
Copy link

ianldgs commented Jun 1, 2022

Hm, actually, thinking back, the presence of the access modifier should already indicate it's not renaming

constructor({ a: b }) is it renaming a to b or saying that a is of type b?
But
constructor({ public a: b }) is clear that it's declaring a public property a of type b.

unless I'm also missing something

@Akxe
Copy link

Akxe commented Jun 7, 2022

@sandersn what part does need to be discarded. Maybe if we knew all that needed to be decided beforehand we could create a poll to vote on the best solution. I know it has not have been done here before, but it could provide great result :)

@ayoreis
Copy link

ayoreis commented Oct 24, 2022

This is my I workaround with default values for simple classes that don't have methods:

class Attribute {
  name!: string
  value!: string
  
  constructor(args?: Partial<Attribute>) {
    Object.assign(this, { name: '', value: '' } as Attribute, args)
  }
}

And if you want optional-without-defaults properties:

class Attribute {
  name?: string
  value?: string
  
  constructor(args?: Attribute) {
    Object.assign(this, args)
  }
}

@REPTILEHAUS
Copy link

Is using Object.assign(this, data) still the DRY recommended approach to de-structure ?

@Roosteridk
Copy link

Still want this in 2023

@weoreference
Copy link

Hello is there a roadmap for this feature? Would greatly improve the ergonomics of typescript!

@a-tarasyuk
Copy link
Contributor

@Akxe
Copy link

Akxe commented Apr 27, 2023

How about a syntax with a new keyword? The keyword deconstructing can only be used before a constructor class method.

This is an example of the possible syntax:

class Test {
  deconstructing constructor(
    private readonly param1: string;
    protected param2?: number;
    public param3: string;
    param4 = param3;
  ) {
    // param2 is optional and yet can be followed by non-optional parameters
    // param4 is local to the constructor only
  }
}

Some nuances:

  • The parameter list is not wrapped in braces ({ })
  • Every line ends with ; and not , to minify mystification with normal constructor

The proposed syntax:

  • Is not easily confused with normal constructor syntax yet similar enough to be easily readable
  • Does define types for the constructor
  • Does allow for all types of parameters, including local to the constructor

Downsides:

  • Does not allow for the renaming of parameters
  • Does not (currently) handle overloading

And this is the produced code/typescript

class Test {
  private readonly param1: string;
  protected param2?: number;
  public param3: string;

  constructor(options: {
    param1: string;
    param2?: number;
    public param3: string;
    param4?: string
  }) {
    this.param1 = options.param1;
    this.param2 = options.param2;
    this.param3 = options.param3;
    var param4 = options.param4 ?? param2;
    // param4 is local to the constructor only
  }
}

@Tomaszal
Copy link

Tomaszal commented Jul 15, 2023

I really like the solution proposed by @rodrigolive in #5326 (comment). Most of it can even be achieved already by adding branded types to their example (I'm sure this can be improved, suggestions are welcome):

declare const tag: unique symbol;

type ConstructedProp<T> = T & { [tag]: typeof tag };

type ConstructedProps<Class> = Pick<
  Class,
  {
    [Key in keyof Class]: Required<Class>[Key] extends ConstructedProp<unknown>
      ? Key
      : never;
  }[keyof Class]
>;

type ConstructorProps<Class> = {
  [Key in keyof ConstructedProps<Class>]: Required<
    ConstructedProps<Class>
  >[Key] extends ConstructedProp<infer PropType>
    ? PropType
    : never;
};

const constructProp = <T>(prop: T): ConstructedProp<T> => prop as ConstructedProp<T>;

Then, class props can be defined like so:

class FooBar {
  foo: ConstructedProp<number> = constructProp(this.props.foo);
  bar: ConstructedProp<number> = constructProp(this.props.bar * 7);
  baz?: ConstructedProp<number> = constructProp(this.props.baz);
  qaz?: number;

  constructor(private readonly props: ConstructorProps<FooBar>) {}
}

const foobar = new FooBar({ foo: 3, bar: 5 });

Unfortunately, useDefineForClassFields has to be set to false for this to work, which is not ideal, as it strays from the recommended defaults. I don't think a similar solution is currently possible with useDefineForClassFields set to true.

@Akxe
Copy link

Akxe commented Jul 17, 2023

@Tomaszal This is a nice type-safe workaround until a proper solution is in place, but it would break for any generic class, where generic used to be defined by parameter in constructor.

Also a big negative is that you cannot mimic defining class properties in constructor. This results in looking thought whole class declaration for any constructor props that might be present. Also private properties would not be properly found, since they are not visible to other function/types. And lastly class extending would break since the properties of parent would be requested in children too.

@awlayton
Copy link

My current workaround is to create a class decorator that will destructure the first constructor argument "automagically"

/**
 * Class decorator for automagically destructuring the constructor argument
 */
function destructure<T, R extends [...unknown[]]>(
  Klass: Constructor<T, R]>,
  _context?: unknown,
): Class<T, [Partial<T>]> {
  const properties = Object.getOwnPropertyNames(
    new Klass({}),
  ) as unknown as Array<keyof T>;
  return class
    extends // @ts-expect-error classes as variables nonsense
    Klass
  {
    constructor(argument: Partial<T> = {}) {
      const a = [];
      for (const property of properties) {
        a.push(argument[property]);
      }

      super(...a);
    }
  } as unknown as Class<T, [Partial<T>]>;
}

With I can use parameter properties in the class like so:

export @destructure class Foo {
  constructor(
    public foo: string,
    public bar: number,
  ) {}
}

But then I will be able to call the decorated constructor with an object:

  const foo = new Foo({bar: 1, foo: 'hi'});

@kibiz0r
Copy link

kibiz0r commented Apr 11, 2024

@awlayton Having to new up an instance in order to interrogate the fields creates a dangerous situation for constructors that have side-effects.

If I have a singleton pattern like:

export @destructure class Foo {
  static instance: Foo;
  constructor(
    public foo: string,
    public bar: number,
  ) {
    Foo.instance ??= this;
  }
}

...now Foo.instance is an instance which never got a foo or bar argument.

@awlayton
Copy link

Yes that is true @kibiz0r. My solution is only helpful for very specific cases. Language support for it would be much better.

@kibiz0r
Copy link

kibiz0r commented Apr 16, 2024

Worth noting that the this.props pattern suggested by @rodrigolive has suffered some bit rot since they posted it.

A class like this will break if you're using the ES2022 syntax for class fields, which is the default for target ES2022 or higher, including ESNext. For this to work, you'd need to set useDefineForClassFields to false in your tsconfig.

class Foo {
  constructor(private props: { foo: number }) {}
  private foo = this.props.foo;
}

@HasanMothaffar
Copy link

We still want this in 2024

@a-tarasyuk
Copy link
Contributor

@Akxe
Copy link

Akxe commented Jul 24, 2024

@a-tarasyuk Where is the discussion happening? I want to follow that thread.

@a-tarasyuk
Copy link
Contributor

@Akxe you can find the details on why the PR aiming to resolve this issue was closed here #44912 (comment). Alternatively, you can ask @RyanCavanaugh or @DanielRosenwasser for more detailed explanations...

@Akxe
Copy link

Akxe commented Jul 24, 2024

@a-tarasyuk The comment above is in reaction to your [comment in #44912]. (#44912 (comment)). My question is where the mentioned discussion is happening. This thread is almost dead yet there seems to be a lot of interest for this specific feature in the community. If you think that the other leading members should add to this discussion, please welcome them here, or we could even schedule some kind of "meeting" to discuss this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Effort: Moderate Requires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual". In Discussion Not yet reached consensus Suggestion An idea for TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.