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 dynamic names in types #15473

Merged
merged 65 commits into from
Nov 16, 2017
Merged

Allow dynamic names in types #15473

merged 65 commits into from
Nov 16, 2017

Conversation

rbuckton
Copy link
Member

@rbuckton rbuckton commented Apr 29, 2017

This changes allows the use of an Identifier or PropertyAccessExpression as part of a computed property name in an interface, class, or type literal as long as the type of the expression is a string or numeric literal type, or is a unique symbol type.

Unique Symbol Types

A unique symbol type is created in specific cases when calling the global Symbol function or the global Symbol.for function, or when you use the unique symbol type.

Unique symbol types have several rules and restrictions:

  • The unique symbol type may only be used on a const variable or readonly property declaration.
  • The unique symbol type widens to symbol.
  • Two unique symbol types are not assignable to each other.
  • A unique symbol type is bound to the symbol of the declaration where it was defined.
    • This prevents assignability errors when merging declarations of the same variable/property that both define their type as unique symbol, as the names for these declarations merge into the same symbol.
  • NOTE: Even though Symbol.for(x) would return the same ES Symbol at runtime if called multiple times for the same x, we currently treat separate calls to Symbol.for(x) as unique symbols. We may choose to revisit this in the future.

Examples

// main.ts (input)
const x = Symbol();
const y = x;
const z: typeof x = x;
interface SymbolConstructor {
  readonly iterator: unique symbol; 
}
interface SymbolConstructor {
  readonly iterator: unique symbol; // ok
}

// main.d.ts (declaration output)
const x: unique symbol;
const y: symbol;
const z: typeof x;
interface SymbolConstructor {
  readonly iterator: unique symbol; 
}
interface SymbolConstructor {
  readonly iterator: unique symbol;
}

Late Binding

Dynamic member names are resolved and "bound" in the checker on-demand whenever the members of a symbol are requested, allowing members with dynamic names to participate in type relationships.

Since dynamic members are bound later than syntactically recognizable member names, we disallow defining a member both syntactically and via a dynamic name so as not to introduce inconsistencies with overload resolution as the declarations might end up in the wrong order.

Examples

// main.ts (input)
export const x = "literal name";
const y = 1;
export interface A {
  [x]: string;
  [y]: string; // error: Interface 'A' has or is using private name '[y]' (when using --declaration)
}
type B {
  [x]: string;
  [y]: boolean;
}
let a: A;
let b: B;
a = b; // error: Type 'B' is not assignable to type 'A'. Types of property '[y]' are incompatible.

// main.d.ts (declaration output)
export declare const x = "literal name";
export interface A { 
  [x]: string; 
}

Fixes #2012
Fixes #5579
Fixes #7436 (via typeof SAYHELLO)
Fixes #11736 (via typeof opAdd)
Partially Fixes #13031 (via unique symbol type, though there are other issues that still block this)


interface T16 {
[c5]: number;
[c6]: string;
Copy link
Contributor

@gcnew gcnew Apr 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be an error? I also tested with duplicate string literals with incompatible types. I expected a compile time error, however a union may also be possible.

const k1 = "literal name";
const k2 = "literal name";

interface T17 {
  [k1]: string;
  [k2]: 123;  // error expected
}

type T18 = {
  [k1]: string;
  [k2]: 123;  // error expected
}

// strange IntelliSense:
// type T18 = { literal name: string }

interface T19 {
  ['literal name']: string;
  ['literal name']: 123; // error as expected
}

interface T20 {
  ['literal name']: string; // error here
  [k1]: 123;                // and also here
}

let t17: T17;
t17['literal name'] // strange IntelliSense: `T17.literal name: string`

let t19: T19;
t19['literal name'] // strange IntelliSense: `T19[['literal name']]: string`

@sandersn
Copy link
Member

sandersn commented May 2, 2017

Can you give an example of the symbol literal type scenario?

Copy link
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks solid enough, but I don't really get why this change is needed right now. It seems like a lot of change without much payoff. Maybe this should wait until we have symbol literal types?

Also a few nitpicks in the comments.

@@ -0,0 +1,102 @@
// @target: esnext
// @module: commonjs
// @filename: module.ts
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add // @declaration: true here to test the declarationEmitter code?

if (name) {
// TODO(rbuckton): ESSymbolLiteral
const nameType = checkComputedPropertyName(name);
return (nameType.flags & TypeFlags.StringOrNumberLiteral) !== 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally find !! easier to read than !== 0, but I'm not sure how common that is..

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure it has a major impact on performance, but !! requires coercion from SMI to boolean, while !== 0 does not.

}

function resolveDynamicMembersOfClassOrInterfaceOrTypeLiteralNode(node: ClassLikeDeclaration | InterfaceDeclaration | TypeLiteralNode, symbolTable: SymbolTable) {
for (const member of node.members) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it would be simpler to have a third parameter symbol and only one resolveDynamicMembersOfNode(members: NodeArray<ClassElement | TypeElement | ObjectLiteralElementLike>, symbols: Symbol[], symbolTable: SymbolTable). Then resolveDynamicMembersOfSymbol could pull off the properties of node in each case.

@rbuckton
Copy link
Member Author

rbuckton commented May 2, 2017

@sandersn Symbol literal types are a whole other complicated issue. This is basically a rough-in for where symbol literal types would be used, but at least gives us the ability to use string and numeric literal types for dynamic property names now. The branch I am working on that has symbol literal types builds on this, and will help keep the scope of a future PR to just symbol literal specific functionality.

The PR has been updated to include unique symbol types.

@rbuckton
Copy link
Member Author

rbuckton commented May 6, 2017

Updated with support for unique symbol types. Also updated the description, above.

@rbuckton
Copy link
Member Author

rbuckton commented May 6, 2017

At some point we may choose to simplify the binder with respect to well-known symbols and let late-binding take care of it.

@rbuckton
Copy link
Member Author

rbuckton commented Jun 7, 2017

@sandersn any other comments?

@rbuckton
Copy link
Member Author

@ahejlsberg, @mhegazy: In ae11ae5 I've made some changes to how we handle widening in getReturnTypeFromBody to address widening of unique symbol types. This changes how we resolve the return types for async functions, async generators, and generators where it seems like we weren't sufficiently widening literal types in these cases like we were for normal functions. This could be considered a breaking change, so I'd like to know if this is acceptable. If not I can revert to the old behavior, but this seems more consistent.

@nisimjoseph
Copy link

can I use without use "// @ts-ignore" instruction to make the compiler ignore that "mistake"?

for example:

interface HandlerHash
{
    [eventType:string | symbol]:Function[];
}

@HerringtonDarkholme
Copy link
Contributor

@DanielRosenwasser unique symbol is a notable feature in TS2.7 and it can benefit terminal users a lot, e.g., Angular's NgOnInit interface can use this to avoid method name conflict.

Can we add it to What's new in TypeScript wiki? Indeed sometimes TS' new awesome features are more than one can follow. 😜

@mhegazy
Copy link
Contributor

mhegazy commented Jan 24, 2018

@HerringtonDarkholme we are updating the docs currently. we should have them all up-to-date before the final 2.7 goes out next week.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet