wails icon indicating copy to clipboard operation
wails copied to clipboard

TS model constructors are not strongly typed

Open fairking opened this issue 2 years ago • 10 comments

Is your feature request related to a problem? Please describe.

I have a problem with catching compile errors after I change my models in backend. So everywhere in frontend I create models, and I have no control over if there are any issues with passing properties to the constructor. They (models) get generated in TS in the following (untyped) way: models.ts:

export class InitForm {
    owner: string;
    description: string;

    static createFrom(source: any = {}) {
        return new InitForm(source);
    }

    constructor(source: any = {}) {
        if ('string' === typeof source) source = JSON.parse(source);
        this.owner = source["owner"];
        this.description = source["description"];
    }
}

Describe the solution you'd like

I would suggest to generate it in the following format: models.ts:

export class InitForm {
    owner?: string;
    description?: string;

    static createFrom(source: (undefined | unknown) = {}) {
        if (source && typeof source === 'string') source = JSON.parse(source);
        return new InitForm((source || {}) as InitForm);
    }

    constructor(source: InitForm = {}) {
        Object.assign(this, source);
    }
}

Please note the following:

  • owner?: string; // Question mark
  • static createFrom(source: (undefined | unknown) = {}) in some cases you need to pass undefined or unknown values
  • constructor(source: InitForm = {}) strong typed constructor, the compilation errors would prevent from errors.

Describe alternatives you've considered

As alternative it could be useful to have an option to provide my own template to generate models.

Additional context

No response

fairking avatar Nov 15 '23 00:11 fairking

Thanks for opening this! I'm not a TS expert but this looks like a good enhancement! We'll look into it 🙏

leaanthony avatar Dec 03 '23 20:12 leaanthony

It appears this only works if all fields are optional but I'd like to use Go pointers to infer that. Can you think of any improvements to this? How about this?

export namespace main {
    export class Person {
        name: string = ''; // Default value if not provided
        age: number = 0; // Default value if not provided

        constructor(source: Partial<Person> = {}) {
            const { name = '', age = 0 } = source;
            this.name = name;
            this.age = age;
        }

        static createFrom(source: string | object = {}): Person {
            let parsedSource = typeof source === 'string' ? JSON.parse(source) : source;
            return new Person(parsedSource as Partial<Person>);
        }
    }
}

leaanthony avatar Dec 05 '23 08:12 leaanthony

It appears this only works if all fields are optional but I'd like to use Go pointers to infer that. Can you think of any improvements to this? How about this?

👍 I agree, Partial<> would do the job for required fields.

But I also found some cool example from NSwag and how they generate the ts output file:

export class ResetPasswordForm {
    /** Leave empty if the current user is logged in. */
    email?: string | undefined;
    newPassword!: string | undefined;
    /** Used to reset password by providing the correct current password. Leave it empty if the password is being reset within the ForgotPassword endpoint. */
    oldPassword?: string | undefined;
    /** Token used if the reset password performed within the ForgotPassword endpoint. Leave it empty if the old password provided. */
    token?: string | undefined;

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

    init(_data?: any) {
        if (_data) {
            this.email = _data["email"];
            this.newPassword = _data["newPassword"];
            this.oldPassword = _data["oldPassword"];
            this.token = _data["token"];
        }
    }

    static fromJS(data: any): ResetPasswordForm {
        data = typeof data === 'object' ? data : {};
        let result = new ResetPasswordForm();
        result.init(data);
        return result;
    }

    toJSON(data?: any) {
        data = typeof data === 'object' ? data : {};
        data["email"] = this.email;
        data["newPassword"] = this.newPassword;
        data["oldPassword"] = this.oldPassword;
        data["token"] = this.token;
        return data;
    }
}

As you can see it is using newPassword!: string | undefined; for required fields (see explanation mark and undefined). The cons is there are no default values which could be important sometimes. Also I am not fully understand why do they need this check if (data.hasOwnProperty(property)).

Also, would not be more convenient to use Object.assign(this, source); in your example?

fairking avatar Dec 05 '23 12:12 fairking

Object.assign was throwing me errors. Google - it's common. This is backwards compatible so seems better right now?

leaanthony avatar Dec 05 '23 12:12 leaanthony

Object.assign was throwing me errors. Google - it's common. This is backwards compatible so seems better right now?

I haven't had any issues with Object.assign. Maybe this post has an answer: https://stackoverflow.com/a/38860354/1143349

I don't mind but I was simply thinking the generated code would be much smaller because of Object.assign().

fairking avatar Dec 05 '23 12:12 fairking

export namespace main {
    export class Person {
        name: string = ''; // Default value if not provided
        age: number = 0; // Default value if not provided
}

Any thoughts about referenced fields? For example:

type Person struct {
	Name         string
        Description *string
	Age            *int
}
class Person {
  name: string = ''; // Default value if not provided
  description?: string | undefined; // Optional value
  age?: number | undefined; // Optional value

  constructor(source: Partial<Person> = {}) {
    this.name = source.name || '';
    this.age = source.age;
    this.description = source.description;
  }

  static createFrom(source: string | object = {}): Person {
    let parsedSource = typeof source === 'string' ? JSON.parse(source) : source;
    return new Person(parsedSource as Partial<Person>);
  }
}

Please find attached playground link:

fairking avatar Jan 14 '24 12:01 fairking

However I feel the Object.assign would do a better job and it is shorter.

class Person {
  name: string = ''; // Default value if not provided
  description?: string | undefined; // Optional value
  age?: number | undefined; // Optional value

  constructor(source: Partial<Person> = {}) {
    Object.assign(this, source);
  }

  static createFrom(source: string | object = {}): Person {
    let parsedSource = typeof source === 'string' ? JSON.parse(source) : source;
    return new Person(parsedSource as Partial<Person>);
  }
}

Please see the playground.

fairking avatar Jan 14 '24 18:01 fairking

@leaanthony

Please find some proposals https://github.com/fairking/wails/commit/8e24d392a9e5680780686022fad0693b9e208d72

I have found there are a lot of happening around the ts code generation. Have you considered to use some third packages like that one https://github.com/gzuidhof/tygo? However any complex types could break interoperability thought.

fairking avatar Jan 14 '24 19:01 fairking

We use Typescriptify which is a 3rd party package. Considering this is generated code, I'm ok with it not being the most 'correct' or shortest way of doing things - so long as it works. Also, you don't need to use the generated models - they are a working example of what can be done. It's fine to create and import your own models.

Let's concentrate on replacing any with a type and that should be sufficient for this ticket?

leaanthony avatar Jan 14 '24 21:01 leaanthony

@leaanthony

Let's concentrate on replacing any with a type and that should be sufficient for this ticket?

I agree, there is no rush with other packages, but the priority is to make type validation work with typescript. There are other more important things in v3 require attention. Please follow my proposal, if you are happy please feel free to merge it down or have any changes. My intention was to do minimal changes in this ticket.

fairking avatar Jan 14 '24 23:01 fairking