Index signatures for symbols and template literal strings #44512
+3,104
−1,709
Conversation
|
Heya @ahejlsberg, I've started to run the perf test suite on this PR at 8dd8e38. You can monitor the build here. Update: The results are in! |
|
@ahejlsberg Here they are:Comparison Report - master..44512
System
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Shouldn't we allow
|
| // When more than one index signature is applicable we create a synthetic IndexInfo. Instead of computing | ||
| // the intersected key type, we just use unknownType for the key type as nothing actually depends on the | ||
| // keyType property of the returned IndexInfo. | ||
| return applicableInfos ? createIndexInfo(unknownType, getIntersectionType(map(applicableInfos, info => info.type)), |
weswigham
Jun 9, 2021
Member
unknown isn't assignable to string | number | symbol - maybe it'd be better to use any here on the offchance it leaks somewhere in the future?
ahejlsberg
Jun 9, 2021
Author
Member
To ensure property round-tripping, our guiding principle should be what happens when you feed a particular type through a mapped type. Currently, feeding an any through a mapped type produces a string index signature. That used to make sense because it was the widest possible index signature, but you could argue we should equate any to string | number | symbol and have it be shorthand for all three index signatures. That may or may not affect existing code, but we could try.
ahejlsberg
Jun 9, 2021
Author
Member
Oops, comment above was meant for your general question about any and never.
ahejlsberg
Jun 9, 2021
Author
Member
Meanwhile, sure it might be better to use any for the key type for synthetic index infos.
weswigham
Jun 9, 2021
Member
but you could argue we should equate any to string | number | symbol and have it be shorthand for all three index signatures. That may or may not affect existing code, but we could try.
Yeah, it makes sense, imo, for a mapped type over any to make an any index signature, since that'll actually handle symbols. (Just like the any input could have)
| // signature applies to types assignable to 'number' and numeric string literal types. | ||
| return isTypeAssignableTo(source, target) || | ||
| target === stringType && isTypeAssignableTo(source, numberType) || | ||
| target === numberType && source.flags & TypeFlags.StringLiteral && isNumericLiteralName((source as StringLiteralType).value); |
weswigham
Jun 9, 2021
Member
There's also the type
type NumericStrings = `${number}`which might warrant special-casing here, as well? It's a bit awkward because "0.0" is a "numeric string" but is actually a distinct object key string from "0"; but that's already true for the literals themselves, so.... ¯\(ツ)/¯
ahejlsberg
Jun 9, 2021
Author
Member
I'm not sure there's any meaningful special casing we could do for `${number}` because, as you already point out, it may or may not contain a round-trippable numeric string.
src/compiler/checker.ts
Outdated
| if (someType(type, t => !!(t.flags & (TypeFlags.StringOrNumberLiteralOrUnique | TypeFlags.Instantiable)) && !isPatternLiteralType(t))) { | ||
| return grammarErrorOnNode(parameter.name, Diagnostics.An_index_signature_parameter_type_cannot_be_a_literal_type_or_generic_type_Consider_using_a_mapped_object_type_instead); | ||
| } | ||
| if (!everyType(type, t => !!(t.flags & (TypeFlags.String | TypeFlags.Number | TypeFlags.ESSymbol | TypeFlags.TemplateLiteral)))) { |
ahejlsberg
Jun 9, 2021
Author
Member
Nope, TypeFlags.StringMapping is an instantiable type and it was already barred by the test above.
weswigham
Jun 9, 2021
Member
Hm, on inspection,
type A = Uppercase<`${number}`>;Is just
type A = `${number}`;so they just evaporate around patterns (after affecting the concrete bits, even though, conceptually, they should represent a different, smaller, set of strings in the holes), so I guess we don't need to handle them right now. But if we ever change up mappings over/in patterns to be preserved, we'll need to change this to match, I guess.
ahejlsberg
Jun 10, 2021
Author
Member
Yeah, we're good there for now. However, it does appear we don't properly apply the string mapping to template literal types with non-generic patterns. For example, Uppercase<`foo${string}bar`> should become `FOO${string}BAR`, but simply resolves to `foo${string}bar` now. It's unrelated to this PR, but something we should fix.
My understanding from the last design meeting was that we were limiting the new types of index signatures to match the biggest use-cases that we had, keeping room to generalize for unions, finite types, With that in mind, I'm actually surprised to see unions supported in index signatures in this PR. |
|
@typescript-bot perf test this |
|
Heya @ahejlsberg, I've started to run the perf test suite on this PR at a98ff6d. You can monitor the build here. Update: The results are in! |
Unions per se are not the problem as they can trivially be expanded to individual index signatures for each constituent. (Might as well have the convenience of writing |
|
@ahejlsberg Here they are:Comparison Report - master..44512
System
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const x1 = combo['foo-test']; // 'a' | 'b' | ||
| const x2 = combo['test-bar']; // 'b' | 'c' | ||
| const x3 = combo['foo-test-bar']; // 'b' (('a' | 'b') & ('b' | 'c')) | ||
|
|
weswigham
Jun 9, 2021
•
Member
Can we also add a test for
declare var str: string;
const x4 = combo[`foo-${str}-bar`];
const x5 = combo[`${str}-bar`];
const x6 = combo[`foo-${str}`];? Mostly because element access locations aren't literal locations, per sey, in main right now (and that was a change I had to make in my PR).
| // When more than one index signature is applicable we create a synthetic IndexInfo. Instead of computing | ||
| // the intersected key type, we just use unknownType for the key type as nothing actually depends on the | ||
| // keyType property of the returned IndexInfo. | ||
| return applicableInfos ? createIndexInfo(unknownType, getIntersectionType(map(applicableInfos, info => info.type)), |
weswigham
Jun 9, 2021
•
Member
Oh, one other LS-related question this reminds me of (since the composite doesn't preserve declarations in the index info) - when you have
type A = {[idx: `foo${string}`]: {x: any}} & {[idx: `${string}bar`]: {y: any}}
declare var x: A;
const q1 = x.foobar/**/;and you go-to-def on foobar, do we return both index signatures as the definition locations (as I think we should)?
|
@typescript-bot pack this |
|
Heya @andrewbranch, I've started to run the tarball bundle task on this PR at a98ff6d. You can monitor the build here. |
|
Hey @andrewbranch, I've packed this into an installable tgz. You can install it for testing by referencing it in your and then running There is also a playground for this build and an npm module you can use via |
|
One thing that's odd is type Foo = {
[x: string | number]: string;
}desugars to type Foo = {
[x: string]: string;
[x: number]: string;
}even though that's functionally the same as type Foo = {
[x: string]: string;
}right? |
|
@DanielRosenwasser Yes, that is a bit odd, but it's consistent with what happens if you feed |
|
@typescript-bot perf test this |
|
Heya @ahejlsberg, I've started to run the perf test suite on this PR at 8b2098d. You can monitor the build here. Update: The results are in! |
|
@ahejlsberg Here they are:Comparison Report - master..44512
System
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
are literal string unions allowed inside the template string types ? eg: type CoordinateProps = {
[key: `${'x'|'y'|'z'}`]: number;
}; |
No, the type |
|
@typescript-bot perf test this |
|
Heya @ahejlsberg, I've started to run the perf test suite on this PR at d357fa0. You can monitor the build here. Update: The results are in! |
|
@ahejlsberg Here they are:Comparison Report - master..44512
System
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
With this PR we implement support for symbol and template literal string index signatures. We furthermore permit index signature declarations to specify union key types, provided all constituents are either
string,number,symbol, or template literal types with non-generic placeholders. Some examples:An index signature declaration that specifies a union key type is exactly equivalent to a set of distinct index signatures for each constituent key. For example, the
PropertyMapdeclaration above is exactly equivalent to:Index signature declarations are not permitted to specify literal key types or generic key types. Those kinds of types can only be used with mapped types, which map literals key types to distinct properties and defer resolution of generic key types until they're instantiated with non-generic types. For example:
This PR supercedes #26797 which was more ambitious but had overlap between regular properties and index signatures with literal key types that is difficult to reconcile, as well as generic index signatures for which type relationships become exceedingly complex to reason about.
Fixes #1863.
Fixes #26470.
Fixes #42192.