-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
Computed properties (but not known symbols) #1752
Conversation
"category": "Error", | ||
"code": 2466 | ||
}, | ||
"A computed property name cannot reference a type parameter from its contained type.": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
containing type
Conflicts: src/compiler/checker.ts
if (grandparent.kind === SyntaxKind.ClassDeclaration || grandparent.kind === SyntaxKind.InterfaceDeclaration) { | ||
// A reference to this grandparent's type parameters would be an error | ||
if (result = getSymbol(getSymbolOfNode(grandparent).members, name, meaning & SymbolFlags.Type)) { | ||
error(errorLocation, Diagnostics.A_computed_property_name_cannot_reference_a_type_parameter_from_its_containing_type); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you give an example of when this would happen?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, a good example is tests\cases\conformance\es6\computedProperties\computedPropertyNames32.ts:
function foo<T>() { return '' }
class C<T> {
[foo<T>()]() { }
}
I will add this snippet in a comment in the code as well.
if (hasComputedNameButNotSymbol(node)) { | ||
return undefined; | ||
} | ||
Debug.assert(!hasComputedNameButNotSymbol(node)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So ensure that either it does not have a computed name OR if it does, it has a symbol? I'm confused.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OR that it has no name!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be better to invent a term for this, and put a doc comment above the function that defines the term? I could call it a dynamic name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should discuss this; @yuit and I think that the "But" is a problem - it's just "And" but it indicates something that is contradictory in intuition.
hasDynamicName is not bad for starters
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I will change that. You're right, "but" is bad.
Conflicts: tests/baselines/reference/intTypeCheck.errors.txt
Computed properties (but not known symbols)
ES6 allows an arbitrary expression as the name of a property. The syntax is like this:
Here is my current proposal for supporting this feature:
Computed expressions would be allowed in the following places if target is ES6:
Below ES6:
Note that emit for all of the above is straightforward.
Type check:
Questions:
Argument for - we would want this to work:
Argument against - Computed properties would be allowed in class properties, so someone might have a class hierarchy like this: