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

Support type parameterized Deserialize function #9

Closed
1ambda opened this issue Dec 26, 2015 · 8 comments
Closed

Support type parameterized Deserialize function #9

1ambda opened this issue Dec 26, 2015 · 8 comments

Comments

@1ambda
Copy link
Contributor

1ambda commented Dec 26, 2015

I am writing Dserializable abstract class which indicate it can be deserialized and has deserialize Function. It looks like

const cerialize = require('cerialize'); // import

abstract class Deserializable {
  public static deserialize<T> (json, constructor): T {
    return cerialize.Deserialize(json, constructor);
  }
}

// usage
class Command extends Deserializable {
  @deserialize public help: boolean;
  @deserializeAs(CommandOrder, "_order") public orders: Array<CommandOrder>;
  @deserializeAs("_args") public args: Array<String>;
}

let cmd = Command.deserialize<Command>(json, Command);

But since cerialize.Deserialize requires the constructor function as the second arg, It's somewhat verbose.

If we have a type parameterized cerialize.Deserialize function, we can make more simplified Deserialzable. For example,

export abstract class Deserializable {
  public static deserialize<T> (json): T {
    return Deserialize<T>(json);
  }
}

// more simple
Command.deserialize<Command>(json);

Thanks. :)

@weichx
Copy link
Owner

weichx commented Dec 28, 2015

How would you get the run time type information to know what to type to deserialize something into? I'm all for using generics here but since the generic constraints are a compile time feature of Typescript and not reflected in the compiled JS I'm not sure you can make this work in the same way you'd solve this problem in Java or C#.

@weichx
Copy link
Owner

weichx commented Dec 28, 2015

Maybe if you use this it would work but I'd have to try it

export abstract class Deserializable {
  public static deserialize<T> (json): T {
    return Deserialize<T>(json, this.constructor);
  }
}

I'm also not sure that static methods are inherited, so it might need to be more like:

export abstract class Deserializable {
  public deserialize (json : any): this {
    return DeserializeInto(json, this.constructor, this);
  }
}
var cmd = new Command().deserialize(json);

Thats using polymorphic this which landed here: microsoft/TypeScript#4910 but I haven't tried to use this feature before so im not sure if it works as I expect.
This kind of thing really wants to be a mixin, can't wait for that to land in typescript.

@weichx
Copy link
Owner

weichx commented Dec 30, 2015

@1ambda any thoughts here? I'm going to close this otherwise

@1ambda
Copy link
Contributor Author

1ambda commented Dec 31, 2015

@weichx Thanks for detailed explanation and alternatives. But

  1. public static methods are not inherited in TypeScript
  2. polymorphic this also requires type parameters. (It is also verbose)

I also read

but could not found any solution. It seems impossible in TypeScript and this is the limitation of TypeScript as you mentioned.

My final implementation looks like

export interface NoParamConstructor<T> {
  new (): T
}

export abstract class Deserializable {
  public static deserialize<T>(ctor: NoParamConstructor<T>, json): T {
    return cerialize.Deserialize(json, ctor);
  }

  public static deserializeArray<T>(ctor: NoParamConstructor<T>, json): Array<T> {
    return cerialize.Deserialize(json, ctor);
  }
}

Without NoParamConstructor, we have to pass type parameter explicitly. for example,

class Car extends Deserializable {
  // does not work with `deserialize`
  @deserializeAs("engine") public engine: string;
  @deserializeAs("wheels") public wheels: number;
}

describe("Deserializable", () => {
  describe("deserialize", () => {
    it("should parse Car", () => {
      let json = {engine: "M5", wheels: 4};
      let c1 = Car.deserialize(Car, json);
      let c2 = Car.deserialize<Car>(Car, json); // without NoParamConstructor

      expect(c1.engine).toEqual(json.engine);
      expect(c1.wheels).toEqual(json.wheels);
    });
  });
});

One more question @weichx . In this case why should i use deserializeAs instead of deserialize? I found that json.wheels and json.engine are deserilaized to undefined when i used @deserialized.

Thanks :)

@1ambda 1ambda closed this as completed Dec 31, 2015
@1ambda 1ambda reopened this Dec 31, 2015
@weichx
Copy link
Owner

weichx commented Dec 31, 2015

@1ambda how were you using deserialize? I added this PR #13 with your test in it and using deserialize and it passes just fine for me

@weichx
Copy link
Owner

weichx commented Dec 31, 2015

@1ambda Another thought on the above code, it seems like Car.deserialize<Car>(Car, json) is a bit odd. I played with generics a bit more and came up with this:

function DeserializeGeneric<T>(json : any, type : T) : T {
    return <T>Deserialize(json, type);
}

which you should be able to use like this:

var car = DeserializeGeneric(json, Car);

This is probably something I should add to the library, but for now I think its a cleaner solution for your use case than extending an abstract class.

@1ambda
Copy link
Contributor Author

1ambda commented Jan 1, 2016

@weichx I downloaded 0.1.2 and checked that the test is passing as you pointed out. I think the reason why deserialize does not work properly only in my code is, I used customized version of cerialize in node_modules/cerialize which i modified since the previous released version (0.1.1) does not support #11.

In terms of DeserializeGeneric<T>, I agree with your suggestion. I will use it 👍

Lastly, It would be good if you modify Deserialize signature to use generic type T. Current implementations looks like

export function Deserialize(json : any, type? : Function|ISerializable) : any {

As you mentioned, it could be

// If we use NoParamConstructor technique introduced Microsoft/TypeScript#2037 it would be more type-safe.
// For example, `type: NoParamConstructor<T>` instead of just specifying `T`  

export function Deserialize<T>(json : any, type: T) : T {

Thanks :)

@1ambda 1ambda mentioned this issue Jan 1, 2016
@weichx
Copy link
Owner

weichx commented May 5, 2016

This is now supported through GenericDeserialize<T> and GenericDeserializeInto<T>

@weichx weichx closed this as completed May 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants