microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
99.91k stars 12.36k forks source link

Permit type alias declarations inside a class #7061

Open NoelAbrahams opened 8 years ago

NoelAbrahams commented 8 years ago

I normally want my class declaration to be the first thing in my file:

module foo {

  /** My foo class */
  export class MyFoo {

  }
}

So when I want to declare an alias, I don't like the fact that my class has been pushed down further:

module foo {

 type foobar = foo.bar.baz.FooBar;

  /** My foo class */
  export class MyFoo {

  }
}

I suggest the following be permitted:

module foo {

  /** My foo class */
  export class MyFoo {
    type foobar = foo.bar.baz.FooBar;
  }
}

Since type foobar is just a compile-time construct, and if people want to write it that way, then they should be permitted to do so. Note that the type foobar is private to the class. It should not be permissible to export foobar.

mhegazy commented 8 years ago

What scenarios would this enable other than putting it outside the class? it is not expected to be accessible on the class instance/constructor for instance?

NoelAbrahams commented 8 years ago

It's just a way to deal with long-winded namespaces inside a class. No, I wouldn't expect the class to act as a scope for this.

Perhaps #2956 will solve this better. I was just (sort of) thinking aloud here.

mhegazy commented 8 years ago

This looks like a things applicable within a namespace or a function. a class defines a shape of an object rather than a code container, so i would not allow a top-level declarations within the body of a class that does not contribute to the "shape" of the instance of the constructor function.

NoelAbrahams commented 8 years ago

Yes, a class is not a container. It's just for use within the class. I'm not realy sure that I want to defend this ? I just realised that the use case I'm looking at actually requires the type in a value position.

RyanCavanaugh commented 8 years ago

You could do this, which isn't terribly great but isn't horrible either:

module foo {
  /** My foo class */
  export class MyFoo {
      x: MyFoo.foobar;
  }
  namespace MyFoo {
      export type foobar = string;
  }
}
NoelAbrahams commented 8 years ago

@RyanCavanaugh, I agree. That would be the best second choice. I think I've used it a couple of times, but found that positioning code at the bottom of the file a bit disconcerting. I just like the idea of having just the one class per file with no other distractions.

mhegazy commented 8 years ago

The scenario should be handled by the existing type alias declarations outside the class, or using a namespace declaration as outlined in https://github.com/Microsoft/TypeScript/issues/7061#issuecomment-183517330.

Adding a declaration on the class that does not appear on the type does not fit with the current pattern. I am inclined to decline this suggestion, feel free to reopen if you have a different proposal that addresses the underlying scenario.

malibuzios commented 8 years ago

I'm running into this pattern:

I have a generic class and would like to create a type alias that references a specialized generic type expression that is only valid for that particular class scope, for example instead of:

class GenericClass<T> {
  func1(arr: Array<{ val: T }>):  Array<{ val: T }> {
    ..
  }

  func2(arr: Array<{ val: T }>):  Array<{ val: T }> {
    ..
  }
...
}

It would be great if I could alias that complex type expression (Array<{ val: T }>) to a locally scoped type. e.g.

class GenericClass<T> {
  type SpecializedArray = Array<{ val: T }>;

  func1(arr: SpecializedArray): SpecializedArray {
    ..  
  }

  func2(arr: SpecializedArray): SpecializedArray {
    ...
  }
}

I'm not exactly sure how to effectively work around this. Both the solutions provided @RyanCavanaugh and the original one mentioned by @NoelAbrahams would still a require to parameterize the type alias. E.g:

type SpecializedArray<T> = Array<{ val: T }>;

But that's not really what I'm looking for.. The whole idea was to make it simpler and more readable.. (also, if part of the workaround meant I had to use some strange merging between a class and a namespace I would rather just have nothing at all and write all the types verbosely).

malibuzios commented 8 years ago

@RyanCavanaugh @mhegazy

Please consider reopening this issue. Here's a copy-and-paste fragment of real-world code I'm working on that demonstrates the usefulness of a generic type captured into a locally scoped type alias to yield simpler and a more readable type:

export class BrowserDB<V> {
    ...
    set(valueObject: { [key: string]: V }, timestamp?: number): Promise<void> {
        type EntryObject = { [key: string]: DBEntry<V> };

        return PromiseX.start(() => {
            if (timestamp == null)
                timestamp = Timer.getTimestamp();

            let entriesObject: EntryObject = {};
            let localEntriesObject: EntryObject = {};

            for (let key in valueObject) {
                entriesObject[key] = {
                    timestamp: timestamp,
                    key: key,
                    value: valueObject[key]
                }

                localEntriesObject[key] = {
                    timestamp: timestamp,
                    key: key,
                    value: undefined
                }
            }
        ...

Unfortunately today I cannot capture { [key: string]: V } into a type alias that is scoped to the class and captures V, e.g instead of:

set(valueObject: { [key: string]: V }, timestamp?: number): Promise<void> {

Write:

export class BrowserDB<V> {
    ...
    type ValueObject = { [key: string]: V };

    set(valueObject: ValueObject, timestamp?: number): Promise<void> {
       ...
    }
...
mhegazy commented 8 years ago

This could have easily been written as a type alias outside the class, without sacrificing readability or convenience.

type EntryObject<V> = { [key: string]: DBEntry<V> };
class BrowserDB<V> {
  set(valueObject: EntryObject<V>, timestamp?: number): Promise<void> {
  }
}
malibuzios commented 8 years ago

@mhegazy

The idea is that the generic parameter is captured within the alias itself, reducing EntryObject<V> to EntryObject and cleaning up the code a bit. This is perhaps not an ideal example. There could also be situations where there are multiple generic parameters like:

EntryObject<KeyType, ValueType, MyVeryLongClassName>

etc. and then the impact on the readability of the code would be more apparent.

The fact that I did not provide an example with multiple and long named type parameters does not mean that people aren't encountering this, and couldn't benefit from having this in some cases. My example was honest as I simply don't have much code that uses many type parameters and longer name for the type name provided.

Another advantage is that having the alias as local to the class would not 'contaminate' a larger scope, that may have usage of a similar alias. That seems like a basic design principle of type aliases, so it is not really applied to its fullest here.

mhegazy commented 8 years ago

Still do not see that this warrants breaking the consistency of a class enclosure as noted in https://github.com/Microsoft/TypeScript/issues/7061#issuecomment-183514814.

malibuzios commented 8 years ago

@mhegazy

First, thank you for taking note of my request to at least try to reconsider this.

I've read the comment you linked but did not completely understand it. I'm not sure why the concept of a type declaration should be seen as bounded by Javascript scoping and as this is purely a TypeScript meta-syntax that does not relate to any run-time behavior, and removed when code is compiled.

I'm afraid I'm not sufficiently familiar with the compiler code and the intricacies of the design here, but that's basically all I can see from my viewpoint.

mhegazy commented 8 years ago

it is not the compiler. it is more of a question of syntax consistency.

Declarations inside a class body are only constructor, index signature, method, and property declarations. A class body can not have statements, or other declarations. all declarations inside a class body "participate" in the shape of the class either instance or static side. also all declarations inside the class are accessed through qualification, e.g. this.prop or ClassName.staticMethod.

having said that, i do not see a type alias declaration fit with this crowd. and allowing it would be fairly odd. there is value in consistency and harmony in the a programming language syntax, and i do not think the convenience of not writing a type parameter warrants breaking that.

malibuzios commented 8 years ago

@mhegazy

I actually see it as very natural and it is very strange (surprising?) for me to hear it described this way. I think I have a strong conceptual separation between run-time syntax and meta-syntax. I see no problem, functional or aesthetic in having types in this context!

I also see a generic type scoping pattern:

class GenericClas<T, U> {
}

Now, since T and U are types and scoped to the class. Your sort of view would look at this and say, "classes shouldn't have generic parameters because generic parameters do not really fit with the class concept": they are not accessed with this etc.

Since generic parameters are type "aliases" in a broad sense, I see no conceptual difference between them and class-scoped type aliases in this context. They are very similar in the sense that both are notation for a type that is scoped only to the class. It looks very natural for me that aliases would be possible in this particular scope. In any case, since there are already types that can be scoped to a class, adding more does not seem to me to introduce anything novel or even special to me.

Anyway, If the TypeScript is not interested in having class-scoped type aliases, then I guess there's nothing I can do. It's your language and you are the ones who are responsible for it and get paid to improve it. The only thing I can say is that the reasoning here seems very subjective and somewhat arbitrary.

zpdDG4gta8XKpMCd commented 8 years ago

related #2625

consider the following hack that enables what you want (not exactly in a class but very close to it)


export function toInstanceOfMyClassOf<A, B>() {

    type C = A | B;

    return new class {
       public doThis(one: A, another: B) {
       }
       public doThat(value: C) {
       }
    };

}
huan commented 6 years ago

+1

sztomi commented 6 years ago

Here's an example where this feature would be very useful.

export class State<Constant, Trigger> {
  private _transitions: Map<Trigger,
                            Transition<State<Constant, Trigger>, Trigger>>;

  constructor(public value: Constant) {
    this._transitions = new Map<Trigger,
                                Transition<State<Constant, Trigger>, Trigger>>();
  }

could become

export class State<Constant, Trigger> {
  type TransitionT = Transition<State<Constant, Trigger>, Trigger>;
  type MapT = Map<Trigger, TransitionT>;

  private _transitions: MapT;

  constructor(public value: Constant) {
    this._transitions = new MapT();
  }

I'm sorry to see this proposal was declined because a good example was not provided on time. I hope this will be reconsidered.

InExtremaRes commented 6 years ago

+1

RyanCavanaugh commented 6 years ago

The provided example is actually pretty illuminating - type aliases currently can't close over other type parameters. Is the intent that you could (outside the class body) refer to State<Foo, Bar>.MapT ?

zpdDG4gta8XKpMCd commented 6 years ago

@RyanCavanaugh look what you made me do: https://github.com/Microsoft/TypeScript/issues/18074

sztomi commented 6 years ago

@RyanCavanaugh Yes, exactly (and I think the possibility to avoid repetition improves the robustness of generic code like this: if I change the definition of MapT that change will carry over to wherever it's used).

RyanCavanaugh commented 6 years ago

Today classes don't introduce a namespace meaning, and namespaces are always spellable with bare identifiers and don't require the possibility of type parameters. So this would be really quite a large change to just add sugar to do something you can already do today with a namespace declaration and slightly more type arguments.

sztomi commented 6 years ago

Does that also help with repetition?

Viatorus commented 6 years ago

This is really annoying not having this. Please check out C++ using syntax - simple and wonderful.

jcmoyer commented 6 years ago

Here's an example I keep running into:

type StateMachineState = 'idle' | 'walking' | 'running' | 'flying' | 'swimming';
class StateMachine {
  private state: StateMachineState = 'idle';
  ...
}

In this case, I would like to fully encapsulate the use of states within StateMachine. The StateMachineState type should not be available to other code because it is an implementation detail of StateMachine. Currently, the only way to totally hide it (without using a namespace) is to do this:

class StateMachine {
  private state: 'idle' | 'walking' | 'running' | 'flying' | 'swimming' = 'idle';
  ...
}

...which is quite ugly, and I lose the ability to re-use the union type in some internal StateMachine method later on. I would instead prefer to be able to write it like this:

class StateMachine {
  private type State = 'idle' | 'walking' | 'running' | 'flying' | 'swimming';
  private state: State = 'idle';
  ...
  private foo(stateName: State) { ... }
}

I am aware of namespaces, but I think they're overkill for a single class.

ackvf commented 6 years ago

Here is also an example for React setState where alias would be handy: https://github.com/DefinitelyTyped/DefinitelyTyped/pull/25055/commits/38c9802705380d607a030b1240f76f39581977ea#diff-96b72df8b13a8a590e4f160cbc51f40cR289

So instead of

class Component<P, S> {
  setState<K extends keyof this["state"] | keyof S>(
    state: ((prevState: Readonly<this["state"]> | Readonly<S>, props: Readonly<P>) => (Pick<this["state"], K> | Pick<S, K> | this["state"] | S | null)) | (Pick<this["state"], K> | Pick<S, K> | this["state"] | S | null),
    callback?: () => void
  ): void;

we could have

class Component<P, S> {
  type SetStateSignature = Pick<this["state"], K> | Pick<S, K> | this["state"] | S | null
  setState<K extends keyof this["state"] | keyof S>(
    state: ((prevState: Readonly<this["state"]> | Readonly<S>, props: Readonly<P>) => SetStateSignature) | SetStateSignature,
    callback?: () => void
  ): void;

Of course aliasing a reference to this is impossible outside of the class scope.

Ignore the invalid code please, focus on idea.

TomasHubelbauer commented 6 years ago

In my case I have types (which I later use to construct a type union) which all relate to a method, like this:

class Something {
    type A = { type: 'a' };
    methodA() {}

    type B = { type: 'b' };
    methodB() {}

    type C = { type: 'c' };
    methodC() {}

    type Union =
        | A
        | B
        | C
        ;
}

Obviously the above is the preferred syntax, not something we can do today, but I think we should be able to. As for whether the class name should contribute to the type namespace: nope. Just hoist the types out of the class, do not change anything aside from allowing us to place types into classes so they can be next to the related methods when dealing with scenarios like mine.

tplk commented 6 years ago

Probably not a good solution, but you can do it this way:

class A<T, InternalT = {internalProp: number} & T> {
  public data: InternalT[];
}
Wayofthesin commented 6 years ago

If you have a problem with putting it into the class body then don't do it. Consider the following:

export class State<Constant, Trigger>
    alias TransitionT of Transition<State<Constant, Trigger>, Trigger>,
    alias MapT of Map<Trigger, TransitionT>
{
    private _transitions: MapT;

    constructor(public value: Constant) {
      this._transitions = new MapT();
    }

    // ...
}

with inheritance:

export class StringMap<ValueType>
    alias BaseMap of Map<string, ValueType>
    extends BaseMap
{
    constructor(base: BaseMap) {
        // build from base
    }
}

Im not really happy with the alias word there. Maybe something like with type would work better.

zpdDG4gta8XKpMCd commented 6 years ago

type declarations within

pleasd do not roll out a halfbaked one silly case solution

zpdDG4gta8XKpMCd commented 6 years ago

or better yet unify namespaces and objects

RyanCavanaugh commented 6 years ago

@aleksey-bykov if we were doing something one-off and silly, it'd be done already 😉 I think you'll like what @weswigham is working on

weswigham commented 6 years ago

function signatures

Function signatures is an.... Interesting one. There's already two lists there, and a third is... Awkward. I don't think there's a reasonable syntax to be had there. At least not in the style I think we all want. But there'd always be someone like this:

type Sig<A, R> = {
    type Pair = [A, R];
    (arg: A, pair: Pair): R;
}

to kinda bridge the gap.

zpdDG4gta8XKpMCd commented 6 years ago

that would be great!

mattmccutchen commented 6 years ago
type Sig<A, R> = {
    type Pair = [A, R];
    (arg: A, pair: Pair): R;
}

FWIW, this approach does not make it possible to declare a generic method with type aliases that depend on the method's type parameters, unless we have some way to say the equivalent of forall A, R. Sig<A, R>. How about:

type Sig = <A, R with type Pair = [A, R], Other = [R, A]>
(arg: A, pair: Pair, other: Other): R;

~This is like declaring Pair and Other as type parameters with defaults, except they don't receive inferences (instead, source types are inferred directly to their definitions) and can't be specified at call sites.~ On second thought, that description wouldn't work for local type aliases that take their own type parameters, which ideally we would support.

aleclarson commented 5 years ago

Would generic namespaces be a good solution?

// Contrived example
export namespace Foo<T> {
  export type U = Partial<T>
}
export class Foo<T> {
  public foo: U
}

Foo<T>.U == Partial<T>

Related: #19728

zpdDG4gta8XKpMCd commented 5 years ago

@aleclarson some more general idea: unify namespaces and objects

@weswigham @mattmccutchen

on the second thought signatures via type aren't enough, quite often i need to declare type right in the signature:

export function rescale<Unit>(
   leftValue: number & In<Unit>,
   pivotValue: number & In<Unit>,
   totalAmount: number & In<Unit> & Delta,
   scale: number & In<Unit> & Per<Pixels>
): {
   scale: number & In<Unit> & Per<Pixels>;
   leftValue: number & In<Unit>
} { ... }

i wish i could alias number & In<Unit> somewhere right in the function declaration, currently my only option is default type parameters, which brings problems of its own:

export function rescale<Unit, Value = number & In<Unit>>(
   leftValue: Value,
   pivotValue: Value,
   totalAmount: Value & Delta,
   scale: Value & Per<Pixels>): { scale: Value & Per<Pixels>; leftValue: Value} { ... }
mattmccutchen commented 5 years ago

@aleksey-bykov ISTM you're saying the same things I said here.

zpdDG4gta8XKpMCd commented 5 years ago

you might be right, didn't appear to me this way

ugputu18 commented 5 years ago

I found a workaround. Look:

interface AbstractWheel<T> {...}

class AbstractCar<T> {
   Wheel: AbstractWheel<T> // Just to use the type
}

class ElecroCar extends AbstractCar<Electro> {...}

const wheel: ElectroCar['Wheel']
aleclarson commented 5 years ago

Would everyone say this is the ideal behavior for types as pseudo-properties of class objects?

// This type will be shadowed by `Foo<T>.U` in the `class` block
type U = any

// The `U` in `Bar<U>` is taken from inside the `class` block
export class Foo<T> extends Bar<U> {

  // `U` is undeclared outside this `class` block
  type U = Partial<T>

  // `S` can be used from other modules (because Foo is exported)
  static type S = T | null

  // `Foo<T>.` is an implied prefix
  public foo: U
  public sudo(): S
}

// `Foo<T>.` is required outside the `class` block
type S<T> = Foo<T>.S

This proposal grants namespace abilities to class blocks, with the added benefits of (A) less boilerplate, (B) class-scoped internal types, and (C) access to generic parameters.

Anything wrong with this proposal? Is there another proposal being worked on?

cc @weswigham

zpdDG4gta8XKpMCd commented 5 years ago

@aleclarson #8358 might be relevant

aleclarson commented 5 years ago

@aleksey-bykov That issue mentions namespaces-as-values and generic namespaces, neither of which seems relevant to the proposal above. I don't personally mind keeping namespaces and objects separate.

Arlen22 commented 5 years ago

I think this is the closest thing we will get to it.

private static typeValueKeys: "register-email" | "register-password" | "register-captcha";
onChangeTextValues: { [K in typeof UserPage["typeValueKeys"]]: string } = {} as any;
alxarch commented 5 years ago

Another idea would be to use type casts to type parameters.

class Foo<K, V=string, M as Map<K,V>> { ... }
function Bar<T, A as Array<T>> { ... }

A type parameter that uses as must be fully defined by unbound type parameters.

alxarch commented 5 years ago

On second thought @mattmccutchen 's with solution is even better separating type parameters and type parameter aliases clearly while keeping a familiar = syntax. Alternative to with could be type which already defines type aliases and there's no chance it has been used as a type parameter name.

class Foo<K, V=string type M=Map<K,V>, S=Set<V>> {...}

or be more verbose and require type before every alias with the added benefit of not requiring a specific order of declarations for complicated multiline definitions:

class Foo<K, V=string, 
type M=Map<K,V>, 
type S=Set<V>,
type VK=Iterable<keyof V>> {...}

I think it's important to keep the same scope rules as normal type parameters so the mental model is clear.

simeyla commented 5 years ago

It's not clear from much of the conversation above that you can already define a 'local type' INSIDE a function.

The type is scoped to that function but can 'leak' out if returned.

I wanted to define a new 'local' type in order to enforce a common return type of RxJS switchMap() when different code paths returned different types. By defining a 'local type' I was able to do the following:

 return this.address$.pipe(switchMap(address => {

      // define local type that enforces its shape on the different return statements below
      type addressValidationType: { success: boolean, errors?: string[], address?: typeof address };

      if (address.countryCd == 'US' && validateAddress(address) == false) 
      {
           return of<addressValidationType>({ success: false, errors: ['Address incorrect!'] });
      } 
      else 
      {
           return of<addressValidationType>({ success: true, address: address });
      }
 }),
 switchMap(validationResponse => {

      // validationResponse here is of type addressValidationType!
      // this allows different return statements above to return different types as long as they're
      // compatible with the defined type. so this gives me good compiler feedback.
 }));

If the above statement is returned by another function, then the return type is the new named type. VSCode will even show the named type - even though it is 'breaking' scope. You can't declare anything of that type outside the function though.

What's also useful to me is that it's possible to define a property on the type alias that references a parameter to that function (address?: typeof address). So what I'm doing here is 'snagging' whatever type this property is and using it to define my type alias. Then if the type of address changes everything ripples through.

But I would definitely like to be able to declare a type at the type-level, specifically right now for one that depended on the generic parameters (for cleverness with angular resolved data).

xaviergonz commented 5 years ago

The best workaround I could come up with is

class GenericClass<T, AofT extends Array<{val: T}> = any> {
  func1(arr: AofT):  AofT {
    ..
  }

  func2(arr: AofT):  AofT {
    ..
  }
...
}

I wish I could just write something like

class GenericClass<T, type AofT = Array<{val: T}>> {...}

or the like

Rush commented 5 years ago

This is a major pain for writing readable advanced template code.