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

Class Constructors (for abbreviated Knockout model-style typescript adaptations) #479

Closed
ghost opened this issue Aug 19, 2014 · 10 comments
Closed
Labels
Declined The issue was declined as something which matches the TypeScript vision Suggestion An idea for TypeScript Too Complex An issue which adding support for may be too complex for the value it adds

Comments

@ghost
Copy link

ghost commented Aug 19, 2014

Knockout models, which are very concise in javascript, are obnoxious to define in typescript (one of the few things that gets longer in typescript). I'm suggesting the following syntax to make it much cleaner. (Perhaps this is a duplicate?)

This syntax adds a constructor signature with parameters which can be referenced in the field initializers. This is to replace javascript's easy this.someNewProperty = '...' legitimacy while retaining the strong typing information of typescript (that is, not resorting to a cast as any, etc)

class koSubModel(node: any = {}) {
    public a = ko.observable<number>(node.a || 0);
    public b = ko.observable<string>(node.b);
}

class koModel(node: any = {}) {
    public one = ko.observable<number>(node.one || 0);
    public two = ko.observable<string>(node.two || '[missing data]');
    public three = ko.observableArray<number>(node.three || []);
    public four = ko.observableArray<koSubModel>((node.four || []).map(e => new koSubModel(e)));
}

// example usage
$.getJSON('...', data => ko.applyBindings(new koModel(data)));

The generated model seems pretty much what you'd write by hand:

var koSubModel = (function () {
    function koSubModel(node) {
        if (typeof node === "undefined") { node = {}; }
        this.a = ko.observable(node.a || 0);
        this.b = ko.observable(node.b);
    };
    return koSubModel;
})();

var koModel = (function () {
    function koModel(node) {
        if (typeof node === "undefined") { node = {}; }
        this.one = ko.observable(node.one || 0);
        this.two = ko.observable(node.two || '[missing data]');
        this.three = ko.observableArray((node.three || []));
        this.four = ko.observableArray((node.four || []).map(function (e) { return new koSubModel(e); }));
    };
    return koModel;
})();

The alternative (current way) is that all the fields have to be declared and separately assigned in a constructor:

class koSubModel_ {
    public a : KnockoutObservable<number>;
    public b : KnockoutObservable<string>;

    constructor(node : any = {}) {
        this.a = ko.observable<number>(node.a || 0);
        this.b = ko.observable<string>(node.b);
    }
}

class koModel_ {
    public one : KnockoutObservable<number>;
    public two : KnockoutObservable<string>;
    public three : KnockoutObservableArray<number>;
    public four : KnockoutObservableArray<koSubModel_>;

    constructor(node: any = {}) {
        this.one = ko.observable<number>(node.one || 0);
        this.two = ko.observable<string>(node.two || '[missing data]');
        this.three = ko.observableArray<number>(node.three || []);
        this.four = ko.observableArray<koSubModel_>((node.four || []).map(e => new koSubModel_(e)));
    }
}
@ghost ghost changed the title Typescript Primary Constructors (for abbreviated Knockout models, at least) Class Constructors (for abbreviated Knockout models-style typescript adaptations) Aug 19, 2014
@ghost ghost changed the title Class Constructors (for abbreviated Knockout models-style typescript adaptations) Class Constructors (for abbreviated Knockout model-style typescript adaptations) Aug 19, 2014
@danquirk
Copy link
Member

This is roughly some sort of 'primary constructor' syntax with initialization (ex http://stackoverflow.com/questions/22908005/primary-constructors-in-c-sharp-vnext).

While certainly a nice feature in general I don't think this adds enough value for the risk it presents at the moment. It's entirely possible that a future version of EcmaScript adds some similar but different pattern for primary constructor type behavior which would present real issues for TypeScript compatibility. In the end the TypeScript syntax here is no more verbose than what a great many other languages use for this kind of pattern.

Note from the ES6 class proposal:
http://wiki.ecmascript.org/doku.php?id=strawman:maximally_minimal_classes

  • There is (intentionally) no direct declarative way to define either prototype data properties (other than methods) class properties, or instance property
  • Instance property an be created within the constructor body.
  • Class properties and prototype data properties need be created outside the declaration.

@ghost
Copy link
Author

ghost commented Jan 28, 2015

Then can TypeScript have a way of adding fields from within the constructor, without having to declare their type explicitly outside the constructor or in an interface?

@RyanCavanaugh
Copy link
Member

You can write something like this:

class Foo {
  constructor(node: string); // Public signature
  constructor(node: string, // Implementation signature with parameter properties
      public myObservable = ko.observable(node),
      public thing = node.length,
      private otherThing = 0) {
     // Additional ctor logic here
  }
}

@ghost
Copy link
Author

ghost commented Jan 28, 2015

OMG! That is so baller

Thanks!

@ghost
Copy link
Author

ghost commented Jan 28, 2015

So here's an updated example of what you can do with that.

/// <reference path="typings/jquery/jquery.d.ts" />
/// <reference path="typings/knockout/knockout.d.ts" />

module KOModels {
    export class Example {
        constructor(data?: any)
        constructor(
            data: any = {}, // raw javascript object or Example to clone
            unwrapped = ko.utils.unwrapObservable(data), // unwrap for cloning
            public fname = ko.observable<string>(unwrapped.fname || ''),
            public lname = ko.observable<string>(unwrapped.lname || ''),
            public children = ko.observableArray<Example>((unwrapped.children || []).map(e => new Example(e)))
        { }

        public static Assume(target: Example, data: any = {}) {
            data = ko.utils.unwrapObservable(data);

            target.fname(data.fname || '');
            target.lname(data.lname || '');
            target.children((data.children || []).map(e => new Example(e)));
        }
    }
}

var x = new KOModels.Example({
    fname: 'Mommy',
    lname: 'Family',
    children: [
        { fname: 'Alice', lname: 'Family' },
        { fname: 'Bobby', lname: 'Family' },
        { fname: 'Chuckie', lname: 'Family', children: [{ fname: 'Donna', lname: 'Family' }] }
    ]
});

console.log(x.fname());

KOModels.Example.Assume(x, { fname: 'Blah', /* ... */ });

@ghost
Copy link
Author

ghost commented Jan 28, 2015

Is it a bug that public lname = ko.observable<string>(unwrapped.lname || new HtmlButtonElement()) as a parameter does not generate a type exception about new HtmlButtonElement() not being convertable to type string? Similar code as a declared member does generate a type error.

@RyanCavanaugh
Copy link
Member

That's not an error because unwrapped.lname is of type any because unwrapped is of type any because it goes through a call to unwrapObservable where data is of type any. You'd need unwrapped to be of type e.g. { lname: string; } to get an error on that line.

@ghost
Copy link
Author

ghost commented Jan 28, 2015

Yes, I understand that lname has type any, but ko.observable<T>(a : T) accepts a T as its argument, not just any. Shouldn't it be a special case where if the value to a can only be inferred as any (mainly, that it cannot be inferred) then it passes, but if the value type is resolved to something that is specifically known to be incompatible with T, then it should be an error?

@RyanCavanaugh
Copy link
Member

The typechecker only works "bottom-up". I agree that we could in principle detect this, e.g.

var x: string;
var y: any;
var z: number = y || x; // Shouldn't be allowed, in theory

But we'd either need to not collapse union types with any, or have special rules about syntactic forms (e.g. y || x is only assignable if both y andx` are assignable). Neither of those is a small change and should probably be handled in a separate issue.

@ghost
Copy link
Author

ghost commented Jan 28, 2015

Well, thanks. I'll leave that to you to know whether that issue already exists under different wording or how to explain the essence of it in a new issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Declined The issue was declined as something which matches the TypeScript vision Suggestion An idea for TypeScript Too Complex An issue which adding support for may be too complex for the value it adds
Projects
None yet
Development

No branches or pull requests

2 participants