microsoft / TypeScript

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

Support type-checking of computed properties for constants and Symbols #5579

Closed IgorMinar closed 6 years ago

IgorMinar commented 8 years ago

On Angular we are considering using ES6 computed prototype methods as part of Angular’s component api but we realized that typescript doesn’t allow us to express this in a type-safe way.

I’m wondering if this is a feature that could be added to typescript or if there is a fundamental issue that would prevent typescript from supporting this. Example:

{Component, onInit, OnInit} from ‘angular2/core';

@Component({
  selector: 'my-component'
})
class MyComponent implements OnInit {
  [onInit](param1, param2){ return something; }
}

Where onInit is a const string or ES Symbol, and OnInit is an interface with the signature of the [onInit] method. onInit is a an optional life-cycle hook that Angular will call if the component has this property name.

The reason why we find this api style attractive is that it removes the possibility of name-collisions, which means that we are free to add support for more hooks in the future without affecting any existing components.

PS: ES Observables are already using Symbols to define the observable interface, so this feature would allow creation of Observable interface. I expect more libs will want to take advantage of this api style as computed property and Symbol support becomes better.

Related issue: #4653

mhegazy commented 8 years ago

4653 is much simpler to do. in fact @vladima has a change out for this one.

Using non-literals is a different issue. there is a more elaborate discussion here: https://github.com/Microsoft/TypeScript/issues/2012

ahejlsberg commented 8 years ago

@IgorMinar The tricky issue here is that during the binding phase of compilation we need to know the spelling of every symbol (so we can build symbol tables). However, we can't know the actual spelling of the string or symbol that onInit references without resolving that expression, which requires binding information, which is what we're in the process of collecting. So, to support full checking of user defined symbols we'd have to introducing some sort of iterative partial binding which would be a fairly disruptive change. It currently works for built-in symbols only because we specially recognize symbols of the form [System.XXX], but it isn't clear that we could do something similar for libraries in general.

benlesh commented 8 years ago

This seems directly related to an issue I have here: #8099

benlesh commented 8 years ago

@ahejlsberg would it be possible to type this with an annotation like [Symbol.for('xyz')]? Since Symbol.for() creates and returns a single static reference, this could be "type checked" so to speak.

I think there's an obvious problem with dealing with [randomSymbol] properties, in that Symbol('foo') === Symbol('foo') is false.

A weirder problem exists around the polyfilling of symbols with "@@mysymbol"... which depending on the platform is interchanged with a symbol as a property key. so { ["@@mysymbol"]: () => {} } should match { [Symbol.for('mysymbol')]: () => void }.

DanielRosenwasser commented 8 years ago

If there was something like a generic primitive type, symbol<"xyz"> would be an interesting possibility. Default type arguments could also make symbol equivalent to symbol<any>. I dunno, just an idea.

kitsonk commented 8 years ago

Coming in with something related... @ahejlsberg you say that the compiler has a bit of a challenge with resolving the types that aren't on the global Symbol. Even though clearly I don't know the inner working, I wonder if that is strictly still true. For Dojo 2 we produced a Symbol shim which doesn't modify the global scope if there is no Symbol present (though it does fill in missing "well knowns"). Then other modules, instead of relying upon the global Symbol reference the module (therefore minimising issues other code running in the same environment).

When we target es5 everything works perfectly fine. The compiler handles everything and is even aware of the primitive type of symbol. It is only when targeting es6 does the error 'Symbol' reference does not refer to the global Symbol constructor object. present itself.

mhegazy commented 8 years ago

The way it works today is by checking the text of the name to identify the property, that is exactelly "Symbol.iterator". if you change any thing about this it is not found. this is why it is important to know that you are using the global "Symbol" and not a local Symbol. in --t es5, the definition of Symbol does not exist, so the check does not happen. but that does not mean it "works". for instance, see:

import sym from './Symbol';
var i: Iterable<string>;
i[sym.iterator] // not the same as Symbol.iterator.

import Symbol from `./Symbol`;
const iteratorSymbol = Symbol.iterator;
i[sym.iterator] // still not the same as Symbol.iterator.

This issue tracks making it work, ideally by knowing the "identity" of a symbol and use that to define and access properties.

there is more relevant discussion on why it works this way and what is missing in #2012

benlesh commented 8 years ago

the global "Symbol" and not a local Symbol

It's probably worth disambiguating this a bit:

  1. Symbol('description'): A local symbol with a description, always a different reference.
  2. Symbol.for('name'): A global symbol looked up by name and created if it doesn't exist
  3. Symbol.iterable, Symbol.toStringTag, et al: Global symbols that should always be present.
  4. "@@whatever": A string property name used as a place holder, generally for number 2 above, in environments where Symbol doesn't exist (legacy browsers, etc).

I don't think we can track the first type there. The rest, though, should be doable.

kitsonk commented 8 years ago

Ok, I understand now. I guess though, it is a but "surprising" that unlike most features in TypeScript which have some sort of the "trust me, I know what I am doing" flag. This particular one doesn't seem to have such an option.

shlomiassaf commented 8 years ago

Partially related... To really boost the usage of Symbol I would love to see dot notation support.

The developer will define the symbol using a special TS syntax. TypeScript will convert it to an index assignment in the transpilled code.

const mySymbol = Symbol('My Symbol!!!');

class MyClass {
  @mySymbol(value1: string): number { // also supports modifiers.
     // do something...
  }
}

let myClass = new MyClass();
myClass.@mySymbol('abc'); // <- Intellisense + return type number.

Of course there are issues, this is just a quick sample for an idea:

  1. No reference to mySymbol outside of the module
  2. TypeScript unique syntax (like this in functions)
  3. Might cause confusion with decorators at the class declaration level.
  4. Usage when set in 3rd party package imported to a project???

Since (1) is a big issue TypeScript can work with Symbol.for() and auto set the key using module id + the name of the class + name of the property/method and maybe a TS unique prefix. Or just a random string typescript tracks.

class MyClass {
  @mySymbol(value1: string): number { // also supports modifiers.
     // do something...
  }
}

Becomes:

var MyClass = (function () {
    function MyClass() {
    }
    MyClass.prototype[Symbol.for('TS_SYMBOL: 12.MyClass.mySymbol')] = function (value1) {
        // do something...
    };
    return MyClass;
}());

As for (4), open for discussion :)

Anyway, if this is possible it will make Symbols a great feature that can be used in public API's. Imagine a data access object and the model as one class. having @save() operations and other hooks/methods/validation etc without compromising due to naming collisions with the model and not have to use a getters/setters

tomdale commented 8 years ago

I am running into this limitation in my current project. Specifically, I am implementing a JSON serializer that needs to retrieve information about models, such as their ID, attributes, relationships, etc.

Instead of hard-coding the serializer to a particular model library, I wanted to support a serialization protocol object that any model instance can implement, using symbols. Unfortunately the inability to define an interface using a non-built-in Symbol is stymying that somewhat.

The code looks something like this:

namespace JSONAPISerializer {
  export const PROTOCOL = Symbol("serializer-protocol");

  export interface SerializerProtocol {
    getType(): string;
    getID(): string | number;
  }

  export interface Serializable {
    [PROTOCOL]: SerializerProtocol;
  }
}

I want to be able to write a Model class that implements the serialization protocol:

import { PROTOCOL, Serializable } from "json-serializer";

class Model implements Serializable {
  [PROTOCOL] = {
    getType() { ... },
    getId(): { ... }
  }
}

In my case, I am happy to have the constraint of the symbol being const—redefining symbols at runtime seems like a bad idea anyway.

calebmer commented 7 years ago

The implications of this proposal and cross-class privacy interests me. In some more complex systems it may be nice to have a module with multiple classes that can call certain methods on each other, but not allow consumers access to those methods. A hidden symbol (hidden from consumers at least) could be an interesting solution to this. Take the following code:

namespace X {
  const privateMethod = Symbol()

  export class A {
    b: B

    constructor (b) {
       this.b = b
    }

    b (): string {
      return this.b[privateMethod]()
    }
  }

  export class B {
    [privateMethod] (): string {
      return 'yay!'
    }
  }
}

const b = new X.B()
const a = new X.A(b)

// Works
a.b()

// Auto-completion shows nothing when typing `b.` or `b[`
dead-claudia commented 7 years ago

This would make polyfilling proposed well known symbols for proposals like Observables (using Symbol.observable) and async iterators (using Symbol.asyncIterator) possible. Most notably, RxJS 5 itself needs Symbol.observable to properly type Observable.from, because that's used as an interop point, and is mandated in the ES observable proposal. Other libraries that interoperate with observables also are aware of it, like most.from(observable) from most.js.

acutmore commented 7 years ago

@calebmer

A hidden symbol (hidden from consumers at least)

Unique Symbols certainly make access to methods harder but they can't be fully hidden from a consumer. Object.getOwnPropertySymbols will expose an symbols used as properties. They are better suited to prevent clashes without having to namespace properties.

Closures are the only way to block access to something in Javascript. (AFAIK)


+1 For supporting run-time created symbols in interfaces. Well-known symbols and Symbol.for should have the same compiler support so as not to block the functionality of 3rd party libraries such as RxJs.

PyroVortex commented 7 years ago

Symbols are interesting in that the specific type of a runtime created symbol is self-referential:

const key = Symbol();
let x: typeof key = key; // This is the only strictly correct type specifier for the value
let y: typeof key = Symbol(); // This should be a compiler error, as the expression has a different type than the variable.

Edit: corrected new Symbol() to Symbol();

benlesh commented 7 years ago

@PyroVortex Symbol isn't a constructor.

PanayotCankov commented 7 years ago

Hi,

In the NativeScript we have a property system that allows properties that support CSS, inheritance through the visual tree and bindings, be easily defined. A CSSProperty class would put in order the logic for CSS value parsing, validation, change notifications and propagation from the JavaScript visual tree to the underlying iOS or Android visual tree. So we end up having something like:

declare module "ui/core" {
  export class Property<T> {
    readonly setNative: symbol;
    readonly getDefault: symbol;
    readonly descriptor: TypedPropertyDescriptor<T>;
    constructor();
  }
  export class CSSProperty<T> extends Property<T> {
    constructor(args: { cssName: string, parse: (string) => T /* etc. properties based on the type and not the concrete View class */ });
  }

  export class Color {}

  export const colorProperty: CSSProperty<Color>;
  export abstract class View {
    public color: Color;
  }
  // The implementation involves: Object.defineProperty(View.protototype, "color", colorProperty.descritptor);
}

declare module "ui/text-base" {
  import { Property, View } from "ui/core";
  export const textProperty: Property<string>;
  export abstract class TextBase extends View {
    public text: string;
  }
  // The implementation involves: Object.defineProperty(TextBase.protototype, "text", textProperty.descritptor);
}

And then in the actual implementation view classes can alter the behaviour of the properties in nice polymorphic way:

import { colorProperty, View } from "ui/core";
import { textProperty, TextBase } from "ui/text-base";
import { Color } from "ui/color";
class Button extends TextBase {
  [colorProperty.setNative](color: Color): void {
    // Some code that knows how to set the Android's android.widget.Button foreground.
  }
  [colorProperty.getDefault](): Color {
    // Some code that knows how to get the default color considering the native theme from an Android button.
  }
  [textProperty.setNative](text: string): void {
    // Some code that knows how to set Android's android.widget.Button's caption.
  }
}

The implementation of the property's descriptor will call the methods defined using the Symbols. It would be nice if TypeScript could provide type checking for these methods - that the value for the colorProperty.setNative key is a function that takes a Color and return void, and the value for textProperty.setNative key is a function that takes a string and return void. The property objects are exposed as const and the fields as readonly, so it should be possible to compute somewhat fully qualified name for these symbols and be ensured that the symbol instances won't be changed.

jods4 commented 7 years ago

My current project suffers from being unable to type custom Symbol properties on objects.

I read @RyanCavanaugh's answer to this SO question: http://stackoverflow.com/a/41403082 He uses the following example:

// Implementations not visible to us
declare function getSymbol1(): Symbol;
declare function getSymbol2(): Symbol;

const s1 = getSymbol1();
const s2 = getSymbol2();

interface Type1 {
  [s1]: string;
  [s2]: number;
}

And argues that what TS should do is unclear given that s1 and s2 might be the same, different or even random.

I think the following behavior would immensely help real-world use cases: For any object implementing Type1:

Other considerations:

Use case

Using a private symbol is a good way for a library or module to store information on objects without (easily) exposing that information.

The pattern is commonly

// Have a private symbol in my module
const myKey = Symbol("this is where we store our private bits");
// Then store and restore information on objects in the same module
randomObject[myKey] = 42;
// ...
console.log(randomObject[myKey]);

It's unfortunate that this simple pattern cannot be typed with an interface and that all access to [myKey] is any.

axefrog commented 7 years ago

How difficult is this to implement if the following constraints are maintained?

For a user-defined symbol to be used as a compile-time identifier:

1. It MUST be (a) declared as const and (b) created inline as the right operand

// GOOD - no compile-time ambiguity regarding what gets assigned
const a = Symbol('a');

// BAD - compiler has no way to know which symbol is being assigned
const a = getSymbol();

// BAD - compiler can't guarantee that the value of `x.a` is maintained before assignment to `a`
const x = { a: Symbol('a') };
...
const a = x.a;

2. It can ONLY be shared between modules via ES2015-compliant imports and exports

// ---------------------------------------------------------------------------
// a.js

export const a = Symbol('a');

// ---------------------------------------------------------------------------
// b.js

export const b = Symbol('b');
const obj = { x: Symbol('x') };
export const x = obj.x;

export {a} from './a';

// ---------------------------------------------------------------------------
// c.js

import {a, b, x} from './b';
const c = Symbol('c');

export interface C {
  // GOOD - compile-time guarantee via a.js const assignment and pure re-export from b.js
  [a](): void;

  // GOOD - compile-time guarantee via b.js const assignment
  [b](): void;

  // GOOD - compile-time guarantee via internal module assignment
  [c](): void;

  // GOOD - global symbol registry can be evaluated at compile-time as though it were a string
  [Symbol.for('d')]: void;

  // BAD - impossible to guarantee that `x` is unchanged before import is resolved
  [x](): void;

  // BAD - symbol reference is transient
  [Symbol('y')]: void;
}
dead-claudia commented 7 years ago

@axefrog Issues with that:

  1. There's areas where you can guarantee a returned symbol is constant.
  2. You can (and should) allow readonly object members to be used as types. This comes into play with Symbol.* and that of many different libraries.
axefrog commented 7 years ago

@isiahmeadows Could you elaborate on point 1, and offer a revised suggestion with respect to both points?

dead-claudia commented 7 years ago

@axefrog Here's one:

const sym = Symbol("foo")
function getFoo() { return sym }
const foo = getFoo() // Easily proven

And here's how I'd change it:


Eligible values are one the following:

  1. A direct Symbol("literal") or Symbol() call.
  2. A well known symbol.
  3. An expression result that's 1 or 2 (implicitly an alias).
  4. An expression result returning an eligible symbol name's value
// Accepted
Symbol("foo")
Symbol()
const foo = Symbol("foo"); (() => foo)()

// Rejected
Symbol("fo"+"o")
Symbol(ident)
(() => Symbol("foo"))()

Eligible bindings are one of the following

  1. A const binding.
  2. A readonly property within an object tied to a const binding.
  3. A readonly property within an object tied to an object within 2 or 3.
// Accepted
const foo = ...
const ns: {readonly foo: symbol} = ...
const nested: {readonly ns: typeof ns} = ...

// Rejected
let foo = ...
const ns: {foo: symbol} = ...
const nested: {ns: typeof ns} = ...

Eligible symbol names are eligible bindings that return eligible values.

whatisaphone commented 7 years ago

This feature would also be helpful when working with React. A minimal example:

import * as React from 'react';
import { ChangeEvent } from 'react';

interface LoginState { username: string; password: string; }

export class Login extends React.Component<{}, LoginState> {
  public state: LoginState = { username: '', password: '' };

  private onChange(event: ChangeEvent<HTMLInputElement>, property: keyof LoginState) {
    this.setState({ [property]: event.target.value });  // this doesn't work!
  }

  public render() {
    return (
      <form>
        <input value={this.state.username}
               onChange={(e) => this.onChange(e, 'username')}/>
        <input type="password"
               value={this.state.password}
               onChange={(e) => this.onChange(e, 'password')}/>
        <input type="submit" value="Login"/>
      </form>
    );
  }
}

Relevant definition of setState:

class Component<P, S> {
    setState<K extends keyof S>(state: Pick<S, K>, callback?: () => any): void;
}

Right now the commented line in onChange does not typecheck. The compiler doesn't realize that a { [k: keyof LoginState]: string } can be assigned to a Pick<LoginState, K extends keyof LoginState>.

We currently bypass this by calling this.setState({ ...this.state, [property]: event.target.value }), but this essentially bypasses the typechecker (e.g. calling this.setState({ ...this.state, foo: 'bar' }) compiles as well). It's also passing more keys than necessary to setState, and making it do unnecessary work.

mhegazy commented 7 years ago

@johnsoft i am afraid this is a different request and not included in this issue.

whatisaphone commented 7 years ago

@mhegazy You closed issue #15534 as a dup of this one yesterday :). It seems to me it fits within "type-checking of computed properties for constants".

As far as I can tell (and I could be wrong), #15534 is the root cause of the React issue above. What would be the best way for me to get the above issue on the typescript team's radar?

mhegazy commented 7 years ago

i see. this issue are about using a computed property whose type is a single literal type, and that is constant. thus the compiler can make assumptions about the name/key of the property. The example in #15534 made it seem like that is what you were looking for.

The setState example makes it clear what you are looking for. What is needed here is to distribute the type over the union in keyof. i.e. { [property]: event.target.value } would have the type { "username": event.target.value } | { "password": event.target.value }. Do not think this is the same issue. if you want to file a new issue or add more details to #15534 we can reopen it.

whatisaphone commented 7 years ago

Sure, I'll copy my example over to the previous issue.

jacobrask commented 7 years ago

I would want to warn against this.setState({ ...this.state, [property]: event.target.value }) because it might not do what you expect due to setState's asynchronous nature.

Another workaround which does not change your JavaScript output is

this.setState({ [property]: event.target.value } as { username: string } | { password: string });
dead-claudia commented 7 years ago

@jacobrask For the above example, setState works well enough, and in addition, setState is called after the mapped property is resolved, so that's not a concern.

dead-claudia commented 7 years ago

@johnsoft Your onChange was slightly incorrect, so the types would likely be incorrectly inferred. If you add a generic so TypeScript can pass through the information that property only carries one property name, it might work better. See if this fixes your issue:

import * as React from 'react';
import { ChangeEvent } from 'react';

interface LoginState { username: string; password: string; }

export class Login extends React.Component<{}, LoginState> {
  public state: LoginState = { username: '', password: '' };

  private onChange<K extends keyof LoginState>(event: ChangeEvent<HTMLInputElement>, property: K) {
    this.setState({ [property]: event.target.value });
  }

  public render() {
    return (
      <form>
        <input value={this.state.username}
               onChange={(e) => this.onChange(e, 'username')}/>
        <input type="password"
               value={this.state.password}
               onChange={(e) => this.onChange(e, 'password')}/>
        <input type="submit" value="Login"/>
      </form>
    );
  }
}
whatisaphone commented 7 years ago

@isiahmeadows Doesn't work, unfortunately.

$ tsc "-v"
Version 2.4.0-dev.20170518
index.tsx(11,19): error TS2345: Argument of type '{ [x: string]: string; }' is not assignable to parameter of type 'Pick<LoginState, "username" | "password">'.
  Property 'username' is missing in type '{ [x: string]: string; }'.
index.tsx(11,21): error TS2464: A computed property name must be of type 'string', 'number', 'symbol', or 'any'.
dead-claudia commented 7 years ago

@johnsoft The way you were doing it before would've been not type-safe across fields, which is why I suggested the change. I didn't expect it'd solve your problem completely, but it'll make it easier to catch errors (and hit this bug).

whatisaphone commented 7 years ago

@isiahmeadows Right, I get it. It would matter in the case where username and password were not the same type.

nbransby commented 6 years ago

Any update on this? I noticed I still cannot strongly type my non built-in symbol property declarations in 2.6 👎