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

Javascript: Object.assign to assign property values for classes is not respected #26792

Open
ycmjason opened this issue Aug 30, 2018 · 24 comments
Labels
Experience Enhancement Noncontroversial enhancements Suggestion An idea for TypeScript
Milestone

Comments

@ycmjason
Copy link

ycmjason commented Aug 30, 2018

Perhaps TS should be able to identify Object.assign(this, ...) then infer the properties that are assigned to this? Or is this not possible to do? I am not sure.

TypeScript Version:
Version 3.0.3

Search Terms:

  • object assign
  • class
  • does not exist on type

Code
A.js

export default class A {
  constructor ({ x, y, z }) {
    Object.assign(this, {x, y, z});
  }
  f () {
    return this.x;
  }
}

Expected behavior:
TS should be able to identify x as a property of this.

Actual behavior:
Throwing:

Property 'x' does not exist on type 'A'.

Playground Link:
http://www.typescriptlang.org/play/#src=export%20default%20class%20A%20%7B%0D%0A%20%20constructor%20(%7B%20x%2C%20y%2C%20z%20%7D)%20%7B%0D%0A%20%20%20%20Object.assign(this%2C%20%7Bx%2C%20y%2C%20z%7D)%3B%0D%0A%20%20%7D%0D%0A%20%20f%20()%20%7B%0D%0A%20%20%20%20return%20this.x%3B%0D%0A%20%20%7D%0D%0A%7D

Related Issues:
no

Side Note:
I am using TS with Javascript because YouCompleteMe switched to using TSServer for semantic completion. I absolutely love TS so was really happy about the switch! Thanks for the great work!

@ycmjason

This comment has been minimized.

@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript Experience Enhancement Noncontroversial enhancements labels Aug 30, 2018
@RyanCavanaugh RyanCavanaugh added this to the Future milestone Aug 30, 2018
@GongT
Copy link

GongT commented Sep 3, 2018

Why not support return different thing from constructor()?

class X {
	public X: number;

	constructor() {
		return new Y;
	}
}

class Y extends X {
	public Y: number;
}

const obj = new X;
console.log(obj.Y); /// TS2339: Property 'Y' does not exist on type 'X'

It's more common than only Object.assign.

@eritbh
Copy link

eritbh commented Feb 20, 2019

Came here from #28883. I understand that in the general case, the type of the second argument to Object.assign could be of type any, but in my specific case I define that type via an interface beforehand:

interface MyConfig {
    someConfigProp: boolean;
}
class MyClass implements MyConfig {
    // Error: Property 'someConfigProp' has no initializer
    // and is not definitely assigned in the constructor.
    someConfigProp: boolean;
    constructor (config: MyConfig) {
        Object.assign(this, config);
    }
}

It seems to me that comparison between the types of this and config could be done, and if config is some type that's not assignable to this, then that would be the condition where you couldn't determine the type.

The alternative right now is to manually assign every property of the config interface to the corresponding class property, which works but is a pain:

interface MyConfig {
    someProp: boolean;
    someOtherProp: string;
    // ...etc
}
class MyClass implements MyConfig {
    someProp: boolean;
    someOtherProp: string;
    // ...etc
    constructor (config: MyConfig) {
        this.someProp = config.someProp;
        this.someOtherProp = config.someOtherProp;
        // ...repeated for every property of the config object
    }
}

(This whole thing gets even more ugly when dealing when using optional properties on the config object to allow the use of default values:

interface MyConfig {
    // these props are optional in the config object
    someProp?: boolean;
    someOtherProp?: string;
}
class MyClass implements MyConfig {
    // all props are required in the class, with default values
    someProp: boolean = false;
    someOtherProp: string = 'default value';
    constructor (config: MyConfig) {
        if (config.someProp !== undefined)
            this.someProp = config.someProp;
        if (config.someOtherProp !== undefined)
            this.someOtherProp = config.someOtherProp;
    }
}

and as you can see, with lots of properties on the config object, you quickly wind up with a large chunk of code that just does the same job Object.assign normally does, as it automatically handles optional properties. However, because Object.assign interprets a key with value undefined as present, this would be blocked on #13195 for full compatibility.)

Is there any way to achieve this right now? If not, is it viable to implement? I could look into putting a PR together, but no promises as I've never worked with TS's source before.

@GongT
Copy link

GongT commented Feb 22, 2019

@Geo1088 I think you can just use Object.assign(this, config as any), It's really safe in your case

@eritbh
Copy link

eritbh commented Feb 23, 2019

@GongT Thanks for the guidance, don't know why I didn't think to do that myself.

@eritbh
Copy link

eritbh commented Feb 23, 2019

Not quite, though... The issue was never that the arguments were improperly typed, but that the Object.assign call doesn't update which properties are present on its first argument, in this case the constructed instance.

Maybe a better example:

interface MyOptions {
   myRequiredProp: string;
}
class MyClass implements MyOptions {
    myRequiredProp: string;
    constructor (options: MyOptions) {
        // The next line guarantees myRequiredProp is assigned, since
        // it's required on MyOptions
        Object.assign(this, options);
        // Next line generates "Property 'myRequiredProp' is used
        // before being assigned"
        if (this.myRequiredProp === 'some value') {
            // additional setup here
        }
    }
}

@ChristianIvicevic
Copy link

Just stumbled across this issue - I want to assign multiple properties without too much code duplication, but Typescript is complaining when strict flags are set unfortunately.

@eritbh
Copy link

eritbh commented May 9, 2019

Just found this article which outlines a way to accomplish this with the return value of Object.assign(), but doesn't address performing the in-place operation/modifying the type of the first argument without reassignment. https://spin.atomicobject.com/2018/05/14/type-safe-object-merging-2-8/

I'm interested in making a PR for this behavior; does anyone more familiar with the project have any pointers on possible starting points?

@RyanCavanaugh RyanCavanaugh removed the In Discussion Not yet reached consensus label Jul 29, 2019
@glen-84
Copy link

glen-84 commented Aug 15, 2019

Is this a duplicate of #16163?

@eritbh
Copy link

eritbh commented Aug 15, 2019

Looks like it, sorry for not catching that.

I don't know if that is even something you can even express with the current type system

Well this doesn't bode well for my chances making a PR for this, does it...

@glen-84
Copy link

glen-84 commented Sep 3, 2019

Next line generates "Property 'myRequiredProp' is used before being assigned"

You could get around that with a definite assignment assertion (!), but it would be nice if TypeScript could figure that out instead.

@ethanresnick
Copy link
Contributor

This would be amazing. Right now, if you want to augment a POJO with some methods, say, for computing related values, it's really easy to do in JS — but basically impossible to type in TS without a lot of boilerplate or circular reference issues:

class AugmentedPOJO {
  a() { return this.b(); },
  b() { return this.x; }
  constructor(pojo: { x: string }) {
    // doesn't work, as pointed out 
    Object.assign(this, pojo);
  }
}

If we want to guarantee that the new methods (a and b) won't be shadowed by an own prop of the pojo, we could do something in vanilla JS like:

function augment(pojo) {
  return { ...pojo, ...methods };
}

const methods = { 
  a() { return this.b(); },
  b() { return this.x; }
}

But trying to type the this for the functions on the methods objects (which should be { x: string } & typeof methods) blows up with circularity errors that are only avoided when using a class.

The best hack I've come up with is:

type Pojo = { x: string };
type AugmentedTrueType = Pojo & _Augmented;

class _Augmented {
  constructor(pojo: Pojo) {
     object.assign(this, pojo);
  } 
  a(this: AugmentedTrueType) { return this.b(); }
  b(this: AugmentedTrueType) { return this.x; }
}

const Augmented = _Augmented as {
  new (pojo: Pojo): AugmentedTrueType;
};

// Works as expected!
const a = new Augmented({ x: "hello" });

@mfpopa
Copy link

mfpopa commented Apr 22, 2020

What I ended up doing in my case was to merge/augment the class with an interface like below. This is not a solution to the problem but a workaround.

The idea is to have an interface that defines the object passed to the constructor function that has the same name as the class.

export interface Awesome {
  x: string;
  y: number;
  z: boolean;
  zz?: boolean;
}

export class Awesome {
  constructor(props: Awesome) {
    Object.assign(this, props);
  }

  fn() {
    return this.x; // TS finds x on this because of the interface above.
  }
}

If you have a base abstract class that you want to use together with generics to dynamically defines class properties, you can do this:

// eslint-disable-next-line @typescript-eslint/no-empty-interface, @typescript-eslint/interface-name-prefix
export interface BaseAwesome<T extends IBaseAwesomeProps> extends IBaseAwesomeProps {}

export abstract class BaseAwesome<T extends IBaseAwesomeProps> {
  constructor(props: T) {
    Object.assign(this, props);

    // Some contrived logic to show that both "this" and "props" objects contain the desired object properties.
    this.y = props.y > 5 ? 5 : props.y;
  }

  getX(): T['x'] {
    return this.x;
  }

  updateX(x: T['x']) {
    this.x = x;
  }

  abstract logZZ(): void;
}

Then the abstract class can be used like this:

export interface IDerivedAwesomeProps extends IBaseAwesomeProps {
  someNewProp: 'this' | 'that'; // new prop added on top of base props.
  xx: number; // modified base prop to be required and have specific type.
}

export class DerivedAwesome extends BaseAwesome<IDerivedAwesomeProps> {
  logZZ() {
    console.log(this.zz);
  }
}

const awesomeInstance = new DerivedAwesome({
  someNewProp: 'that',
  x: 'some string value',
  xx: -555,
  y: 100,
  z: true,
});

console.log(awesomeInstance.getX()); // -> some string value
awesomeInstance.logZZ(); // -> undefined

@nebkat
Copy link
Contributor

nebkat commented Sep 23, 2020

See #40451 for proposed further improvements and currently possible alternatives

@PLSFIX
Copy link

PLSFIX commented Apr 7, 2021

A workaround solution I used was to create a type alias for the base class. Something like:

interface Opts {
  a: string;
  b: number;
}
export class SomeClass {
  constructor(opts: Opts) {
    Object.assign(this, opts);
  }
}

export type TSomeClass = SomeClass & Partial<Opts>;

At least you get intellisense tips and typechecking

@henhal
Copy link

henhal commented Oct 6, 2022

@PLSFIX This helps somewhat for users of the class, but if class methods use any of the properties of Opts they would still have to be declared as fields, and to have them as readonly the compiler would still complain that they are "definitely not assigned in the constructor". So your solution obviously doesn't help making those properties fields.

If we could have field designators (public/private/protected + readonly) in destructured object parameters this problem would be solved AND we would have the added bonus of being able to initialize parts of an object automatically without having to use Object.assign at all:

interface Opts {
  a: string;
  b: number;
}

export class SomeClass implements Opts {
  constructor({readonly a, readonly b}: Opts) {
  }
}

@asleepace
Copy link

I came across a similar issue where I wanted to essentially extend a type from a third-party library, but didn't want to manually write out all of the properties (lazy 😅). This is what I ended up doing:

Create class from a generic type:

function createClassFromType<T>() {
  return class {
    constructor(args: T) {
      Object.assign(this, args)
    }
  } as ({
    new (args: T): T
  })
}

Create a base class for my Person type:

type Person = {
  name: string
}

const PersonClass = createClassFromType<Person>()

const person = new PersonClass({ name: 'Alice' })

person.name // Alice

Extending PersonClass is also very easy:

class MyPerson extends PersonClass {
  constructor(person: Person) {
    super(person)
  }
  greet() {
    console.log(`Hello, ${this.name}`)
  }
}

Here are some more example use cases:

const classPerson = new PersonClass({ name: 'Alice' })
console.log(classPerson.name)   // Alice

const childPerson = new PersonChild({ name: 'Bob' })
childPerson.greet()   // "Hello, Bob!"

function doSomethingWith(personType: Person) {
  // example method only acceots person type
}

doSomethingWith(classPerson)  // this is ok!
doSomethingWith(childPerson)  // this is ok!

if (childPerson instanceof PersonClass) {
  console.log('person is instanceof PersonClass!')
}

if (childPerson instanceof PersonChild) {
  console.log('person is instanceof MyPerson!')
}

Check it out on the TypeScript playground!

@blaumeise20
Copy link
Contributor

I am having the exact same problem as @eritbh with Object.assign not making TypeScript recognize the assignment to class properties. Please fix this!

@mr-mmmmore
Copy link

mr-mmmmore commented Nov 21, 2023

I've been looking for this for a while and your solution @asleepace is perfectly doing the job!

By directly calling the createClassFromType method when declaring the actual class most of the boilerplate is avoided:

function classFromProps<T>() {
    return class {
        constructor(props: T) {
            Object.assign(this, props);
        }
    } as ({new (args: T): T});
}

type PersonProps = {
    firstname: string,
    lastname: string,
};

class Person extends classFromProps<PersonProps>() {
    get fullname() {
        return [this.firstname, this.lastname].join(' ');
    }
}

const ts = new Person({firstname: 'Tom', lastname: 'Smith'});

console.log(ts);           // ==> Person: {"firstname": "Tom", "lastname": "Smith"} 
console.log(ts.fullname);  // ==> "Tom Smith"

@Malix-off
Copy link

This solution is indeed working,
But at this point, I feel like I'm hacking TypeScript into the language to make it work.
Maybe that is the point where I will switch to full functional

@henhal
Copy link

henhal commented Dec 13, 2023

Any solution except TypeScript itself handling this will be an ugly workaround. Using a class declaration wrapper function every time we want to declare a class is just hideous. Yes, it works, but so does resorting to any or going other pointless routes that don't really address the problem.

@Amiko1
Copy link

Amiko1 commented Dec 27, 2023

It's curious why that problem is not solved? while it is not random bug, I can't use Object.assign for assign Mixin's Property at my Class without type error, I think // @ts-ignore is best solution

@Malix-off
Copy link

@Amiko1
// @ts-ignore is not a solution
It's a shitty workaround

@nnmrts
Copy link

nnmrts commented Dec 30, 2023

Any solution except TypeScript itself handling this will be an ugly workaround.

Especially because this also affects us vanilla JS users out there who just like Code Lens in VSCode.

In normal JS I can't just hack in some interfaces or types, all I got is JSDoc comments and I have no idea how to workaround this with them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Experience Enhancement Noncontroversial enhancements Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests