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

Allow const variables with new signatures to be types #18942

Closed
tinganho opened this issue Oct 4, 2017 · 12 comments
Closed

Allow const variables with new signatures to be types #18942

tinganho opened this issue Oct 4, 2017 · 12 comments
Labels
Declined The issue was declined as something which matches the TypeScript vision Suggestion An idea for TypeScript

Comments

@tinganho
Copy link
Contributor

tinganho commented Oct 4, 2017

I want to make use of inference when defining a class. Thus, I create a function returning a class instead of defining a class the normal way:

function Model<P extends Properties>(p: P) {
    return class {
        static create() {
            create(): /*use of P somehow*/
        }
    }
}

class User extends Model({
    a: { type: Type.String }
}) {};
let user: User = new User();

Though, I think the line class User extends Model() {}; is a bit non-idiomatic-js and ugly. It would be great if I could do this instead:

function defineModel<P extends Properties>(p: P) {
    return class {
        static create() {
            create(): /*use of P somehow*/
        }
    }
}

const User = defineModel({
    a: { type: Type.String }
});
let user: User = new User(); // I can use 'User' as a type here

At the end, this is working in JS, but TS prevents me.

@jcalz
Copy link
Contributor

jcalz commented Oct 4, 2017

So, I was going to say something like this:

You're asking for TypeScript to implicitly define a type with the same name as a value you created, the way it does with `class`. In the absence of that, you can explicitly define that type yourself:
const User = defineModel({
    a: { type: Type.String }
});
type User = typeof User.prototype; // explicit
let user: User = new User(); // works

But unfortunately, the User type is interpreted as something like Model<any>.(Anonymous class). The type parameter P has been erased to any, even though it had already been concretely specified as { a: { type: ... }}. That means if the returned class has any properties dependent on P, the User type will fail to represent it, in which case you might as well resort to that class User extends Model() {} method which has the intended behavior.

This erasure feels like an actual bug or design limitation, but my search-fu wasn't strong enough to identify if there's a reported issue about it. Is it related to #13807? Is this just a casualty of not having higher kinded generics (#9366)?

@tinganho
Copy link
Contributor Author

tinganho commented Oct 4, 2017

Yes, strange that it didn't work with typeof User.prototype. But I think it is slightly worser than class User extends Model() {}. Ideally, would be just const User = defineModel();. const User is the same as class User, so I think we should allow const User to be in a type position as well.

@mhegazy
Copy link
Contributor

mhegazy commented Oct 4, 2017

The issue here is you have no way in the language to refer to the return type of a constructor function. in a class, the class name addresses that, but not for a non-class constructor. #6606 tracks addressing this.

@mhegazy mhegazy added the Duplicate An existing issue was already created label Oct 4, 2017
@tinganho
Copy link
Contributor Author

tinganho commented Oct 4, 2017

@mhegazy I don't think I understand your answer completely. #6606 isn't it different? It will allow you to write typeof new User(), to get the instance type. But I think it is quite tedious to write in this case. The suggestion is to allow people to just write the variable name directly instead of the class name.

const User = defineModel() above is equal to class User extends Model() {}. Why can't User be in the type position in both of the cases?

@aluanhaddad
Copy link
Contributor

aluanhaddad commented Oct 5, 2017

@tinganho your line of reasoning definitely makes sense, but I think that there would be a few issues with changing the current behavior. It would be a breaking change to existing code that explicitly declared a type such as

const User = defineModel();
type User = typeof User['prototype'];
export default User;

There are also scoping issues which could interact oddly with declaration merging. var User vs. const User comes to mind, especially in the context of control flow analysis.

@jcalz
Copy link
Contributor

jcalz commented Oct 5, 2017

@mhegazy Which issue is this a duplicate of? #6606? I don't see how extending typeof would address either the implicit-type-name issue or the problematic-generic-erasure issue mentioned here. We don't need to extend typeof to get an instance type from a constructor value: typeof Ctor['prototype'] works, except for the generic case mentioned above, right? Or are you saying #6606 would solve that? 😕

Maybe to clear things up we should keep this issue for the suggestion to get implicitly defined type names for class constructor constants, and I should create a new one for the problematic-generic-erasure issue that prevents typeof User['prototype'] from working. Or is that a known issue? I'd rather avoid being slapped with the duplicate tag.

@mhegazy
Copy link
Contributor

mhegazy commented Oct 5, 2017

@mhegazy Which issue is this a duplicate of? #6606? I don't see how extending typeof would address either the implicit-type-name issue or the problematic-generic-erasure issue mentioned here. 😕

In the OP, the instance type that @tinganho wants to use is the type of the expression new User(). I believe the core issue here is that there is no way in the language to refer to this type by name.. your only way of doing so is capturing the expression, e.g. const _user = new User(); type User = typeof _user;. #6606 is so far the best solution we have received for this issue. if you have alternative suggestions please feel free to file a new issue.

We don't need to extend typeof to get an instance type from a constructor value: typeof Ctor['prototype'] works, except for the generic case mentioned above, right? Or are you saying #6606 would solve that?

As you noted, typeof Ctor['prototype'] will instantiate the generic type parameters with any. what you need is a full overload resolution and full generic type inference. in #6606 you would write typeof new C(number, string) for instance to instantiate a construct signature like new<T, U>(a:T, b:U) =>O<T, U>.

Maybe to clear things up we should keep this issue for the suggestion to get implicitly defined type names for class constructor constants,

As i noted earlier, values and types are not the same thing. this is a fundamental concept of the TypeScript type system that allows declaration merging for the same declaration name in multiple spaces. Changing what a value vs. type means is first not correct and second would be a breaking change. today declare const X: new()=>o; interface X {} have a meaning, making const X now a type would break that.

@jcalz
Copy link
Contributor

jcalz commented Oct 5, 2017

@mhegazy Thanks for your reply.

values and types are not the same thing ... first not correct and second would be a breaking change

I understand this and agree that implicitly assigning types to const constructor values would be a breaking change. So therefore this issue should either be declined or marked as a suggestion or something, but not marked as a duplicate of #6606, because it isn't one. All #6606 does is provide a workaround. And there's already the workaround of class Ctor extends functionCall().


full overload resolution and full generic type inference.

I guess I still don't understand why typeof User['prototype'] instantiates P with any when User has already instantiated P as { a: { type: ... }}, but I don't want to hijack this issue further. Could someone point me to an existing issue that addresses this? Thanks!

@tinganho
Copy link
Contributor Author

tinganho commented Oct 5, 2017

const _user = new User(); type User = typeof _user;. #6606 is so far the best solution we have received for this issue.

@mhegazy I proposed in my OP to use the name of the variable const User in this case? I also mentioned that there was an already solution to get the instance type with class User extends Model() {}. I think type User = typeof _user; is even more complicated in this case. Though, I would like if we could make it even easier by just using the variable name of const User?

@mhegazy
Copy link
Contributor

mhegazy commented Oct 5, 2017

I guess I still don't understand why typeof User['prototype'] instantiates P with any when User has already instantiated P as { a: { type: ... }}, but I don't want to hijack this issue further. Could someone point me to an existing issue that addresses this? Thanks!

it is instantiated with any first because we do not have a better type to instantiated with. and second, if it can not be instantiated later.
In our type system, type arguments can be specified in a limited set of syntactic locations, that is a type reference var x: Foo<T>, a call/construct expression foo<T>(...) or following an extends expression in a class. Only in these locations can you instantiate a genetic type. So for instance, you can no write something like const C = ..; C<number>.member.
So if prototype for a generic class was not instantiated, you would be left with a C<T> with un-instantiated T, that you have no way to instantiate, and it is not comparable to any other type (since it is a generic type parameter). pretty much unusable. any let's be more tolerable.

I understand this and agree that implicitly assigning types to const constructor values would be a breaking change. So therefore this issue should either be declined or marked as a suggestion or something, but not marked as a duplicate of #6606,

Sorry for the confusing labeling. the suggestion for const introducing a named type is something we can not consider, and should be marked as a declined suggestion.
The underlying issue, as i see it, is one that is tracked by #6606.

@mhegazy I proposed in my OP to use the name of the variable const User in this case? I also mentioned that there was an already solution to get the instance type with class User extends Model() {}. I think type User = typeof _user; is even more complicated in this case. Though, I would like if we could make it even easier by just using the variable name of const User?

This solution, as well as something like const _user = new User(); type User = typeof _user; require you to write an expression just to capture its type, and both are undesirable. ideally you should have a way to tell the compiler i am referring to the instance type of this constructor function.. something that is missing today.

@mhegazy mhegazy added Suggestion An idea for TypeScript Declined The issue was declined as something which matches the TypeScript vision and removed Duplicate An existing issue was already created labels Oct 5, 2017
@mhegazy mhegazy closed this as completed Oct 5, 2017
@tinganho
Copy link
Contributor Author

tinganho commented Oct 5, 2017

I think this was a bit fast declined.

I think this is quite a usual pattern to define a factory function that defines an object in JS. You not only want to use the value of the object everywhere, but also the type. Letting people defining both the value and the type is quite frustrating. Class declarations that extends the return type of a function is a solution, though it looks very hackish. At the end, a variable declaration that gets assigned a class declaration is nearly identical to a normal class declaration. And IMO, more idiomatic JS.

@RyanCavanaugh
Copy link
Member

Problems with creating a type name include:

  • There can be multiple construct signatures, and those construct signatures can return completely disparate types
  • This would be a breaking change as there would be potentially new conflicts in the type namespace
  • Some construct signatures might have different generic arity, which we'd have no way to model
  • There's an architectural problem because we would be creating new bindings mid-typechecking (i.e. we don't know that User should be a type name until after its typechecking), which implies that the checker would have to do a potentially unbounded number of passes.

Consider something like this

type Blah = { };
namespace X {
  var y: Blah = 10; 

  const Blah = someFunc(y);
}

The meaning of Blah at y now depends on the the result of Blah at the bottom of the namespace, which involves checking y. Today we can top-down check this without having to worry about new names being introduced later in the process.

Overall #6606 is the best way to address this scenario

@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
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
Projects
None yet
Development

No branches or pull requests

5 participants