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

Primitive literals in computed property names #4653

Closed
weswigham opened this issue Sep 4, 2015 · 11 comments
Closed

Primitive literals in computed property names #4653

weswigham opened this issue Sep 4, 2015 · 11 comments
Labels
Fixed A PR has been merged for this issue In Discussion Not yet reached consensus Suggestion An idea for TypeScript

Comments

@weswigham
Copy link
Member

Looking at #4648 brought this up -

declare module JSX {
  interface IntrinsicElements {
    ["my-custom-element"]: MyCustomElementClass
  }
}

We should allow string literals in computed property names in interfaces. Right now we only allow well-know symbols, but allowing all primitive literals (of valid indexer types, I suppose) should be supported.

This is a spec change - section 2.2.3 would need to be updated to reflect this, but, IMO, is fairly important and worth the change. We can't strongly type, say, tslint.json result structures right now, as we lack this.

@danquirk
Copy link
Member

danquirk commented Sep 4, 2015

See #1082 for some previous discussion on computed property names in interfaces.

@vladima
Copy link
Contributor

vladima commented Sep 4, 2015

will this work in your case?

declare module JSX {
    interface IntrinsicElements {
        "my-element": __React.HTMLAttributes
    }
}

<my-element autoPlay/>

@weswigham
Copy link
Member Author

Huh. Why does our syntax for defining non-identifier string object properties in interfaces not overlap with the computed property syntax?

That's weird and unexpected.

It's quick info is also really weird:
image

That's... not valid JS in that quick info. It should certainly use square brackets, reflecting it's correct usage.

Yeah, still feels wrong:
image

Also, we don't get completions for this:
image

@vladima
Copy link
Contributor

vladima commented Sep 4, 2015

I guess that it reflects syntax for defining object literals. Also string\numeric literals as names in properties were allowed in TypeScript long before ES6 support was added

@weswigham
Copy link
Member Author

Weird - VSCode's syntax highlighter doesn't even think it's valid:
image

Like, I feel like the square brackets (computed property) should certainly be correct syntax in TS, even if the old interface syntax is still valid - just because it works for symbols, so it feels like it aught to work for other primitives.

@weswigham
Copy link
Member Author

I also don't think many people would argue with me that
image
is better written as
image

The top is feels almost nonsensical (I did not even realize that was valid TS until I tried it), the bottom is ES6 classes (and yet invalid in TS).

@danquirk
Copy link
Member

danquirk commented Sep 4, 2015

Both of those forms are valid in classes (where someone would actually migrate JS to TS). Would be reasonable to support both in interfaces for consistency sake.

@mhegazy mhegazy added Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels Sep 18, 2015
@jbondc
Copy link
Contributor

jbondc commented Sep 24, 2015

Also was confused by this recently. Consider #2225
image

Don't really know if string literals should be in computed property names but you should be able to see those strings somewhere.

@DanielRosenwasser
Copy link
Member

@jbondc I'm not really sure how #2225 ties in, but #606 is relevant.

@weswigham
Copy link
Member Author

@DanielRosenwasser in the chrome devtools, one can write

obj.t/**/

get completions at the marker, and recieve the list of properties

thing
thing-explainer
$thing
other-thing

select thing-explainer, and have it completed as

obj["thing-explainer"]/**/

I think this is the behavior we want to be able to replicate. (as far as completions go)

@vladima
Copy link
Contributor

vladima commented Nov 10, 2015

should be addressed in #5535

@vladima vladima closed this as completed Nov 10, 2015
@vladima vladima added the Fixed A PR has been merged for this issue label Nov 10, 2015
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Fixed A PR has been merged for this issue In Discussion Not yet reached consensus Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

6 participants