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

typescript: field initialization order in derived classes #625

Open
wuzzeb opened this issue Feb 15, 2018 · 3 comments
Open

typescript: field initialization order in derived classes #625

wuzzeb opened this issue Feb 15, 2018 · 3 comments

Comments

@wuzzeb
Copy link

wuzzeb commented Feb 15, 2018

With c# code such as

public class Bar {
  public int bar {get;set;}
}

public class BarChild : Bar {
  public int[] values {get;set;}
}

the generated typescript will look something like

export class Bar implements IBar {
  bar: number;

  constructor(data?: IBar) {
    if (data) {
        for (var property in data) {
            if (data.hasOwnProperty(property)) {
                (<any>this)[property] = (<any>data)[property];
            }
        }
    }
  }

  // ... rest of Bar class
}

export class BarChild extends Bar implements IBarChild {
    values: number[] = []; // <------ note the initializer here

    constructor(data?: IBarChild) {
        super(data);  // <--- call to base class constructor
    }

   // ... rest of BarChild class ...

The BarChild constructor causes incorrect data initialization. The rules for typescript classes (and eventually javascript classes once emcascript adds property initializers) is as follows:

  1. The base class initialized properties are initialized
  2. The base class constructor runs
  3. The derived class initialized properties are initialized
  4. The derived class constructor runs

applying these rules to the above, what happens is the base class Bar initialized properties are run (there aren't any), the Bar constructor runs (which copies the properties from data into the new instance), the derived class (BarChild) properties are initialized (in this case, values is set to [] overwriting the value copied from data), and finally the derived class constructor runs (which does nothing). Therefore, no matter what you pass to the BarChild constructor, values will always be the empty list since the initializer overwrites anything from the base class constructor.

You can read more about this at microsoft/TypeScript#10634 or microsoft/TypeScript#1617 or microsoft/TypeScript#13525 Also, https://stackoverflow.com/questions/43595943/why-are-derived-class-property-values-not-seen-in-the-base-class-constructor/43595944 contains a discussion why this is the case.

I was looking through https://github.com/RSuter/NJsonSchema/blob/master/src/NJsonSchema.CodeGeneration.TypeScript/Templates/Class.liquid but I don't see an easy way of fixing... the problem is everything about the base class runs before the derived class even gets a chance to do anything. One option would be to not use property initialization at all. Instead, the BarChild constructor could contain code as

export class BarChild extends Bar implements IBarChild {
  values: number[]; // <--- no initializer

 constructor(data?: IBarChild) {
    super(data);
    if (values === undefined) { // <-- initializer here
       values = [];
    }
  }
}
@RicoSuter
Copy link
Owner

This is pretty serious, thanks for reporting.

I think only your last option would be viable... but it would look like that:

export class BarChild extends Bar implements IBarChild {
  values: number[]; // <--- no initializer

 constructor(data?: IBarChild) {
    super(data);
    if (!data) { // <-- defaults only if no data is provided
       this.values = [];
    }
  }
}

Wouldn't this produce much more code?

@RicoSuter
Copy link
Owner

Can you have a look at my commit? Does it look good? Do you think the issue is fixed with this change?

@rahulbpatel
Copy link

Based on this suggestion, as a workaround I've gone with this

export abstract class ModelBase<T> {

    /**
     * Creates new initialized instance of model
     */
    static new<T extends ModelBase<T>>(this: new () => T, data: any): T {
        const result = new this();

        result.init(data);

        return result;
    }

    protected init(init?: Partial<T>) {
        Object.assign(this, init);
    }
}

export class User extends ModelBase<User> {
   id = 1;
   customInitialized: string;
   protected init(data?: any) {
        super.init(data);

        if (data.something) {
           this.customInitialized = 'foobar';
        }
    }   
}

User.new({id: 2, something: true});

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants