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

Improve support for numeric string types #48837

Merged
merged 3 commits into from
Apr 25, 2022
Merged

Improve support for numeric string types #48837

merged 3 commits into from
Apr 25, 2022

Conversation

ahejlsberg
Copy link
Member

TypeScript represents numeric strings using the type `${number}`. This PR improves support for numeric string types to more accurately reflect that elements of arrays and tuples have numeric string property names. Specifically:

  • Numeric index signatures apply when the index type is ${number}. For example `Foo[][`${number}`] resolves to Foo instead of being an error. This is consistent with our existing rule that index signatures apply when the index type is a numeric string literal. For example Foo[]['0'] already resolves to Foo.
  • In a homomorphic mapped type { [K in keyof T]: XXX }, where T is constrained to an array or tuple type, K has an implied constraint of number | `${number}`.
  • Intersections of type string and a template literal type reduce to just the template literal type. For example, string & `${number}` now reduces to just `${number}`.

Some examples:

function f1(a: boolean[], x: `${number}`) {
    let s = a[x];  // boolean
}

function f2(a: boolean[], x: number | `${number}`) {
    let s = a[x];  // boolean
}

type T1 = boolean[][`${number}`];  // boolean
type T2 = boolean[][number | `${number}`];  // boolean

type Container<T> = {
    value: T
}

type UnwrapContainers<T extends Container<unknown>[]> = {
    [K in keyof T]: T[K]['value']  // Ok, but used to be error
};

declare function createContainer<T extends unknown>(value: T): Container<T>;

declare function f<T extends Container<unknown>[]>(containers: [...T], callback: (...values: UnwrapContainers<T>) => void): void;

const container1 = createContainer('hi')
const container2 = createContainer(2)

f([container1, container2], (value1, value2) => {
    value1;  // string
    value2;  // number
});

For further context, see discussion in #48599.

@ahejlsberg
Copy link
Member Author

@typescript-bot test this
@typescript-bot user test this inline
@typescript-bot run dt
@typescript-bot perf test faster

@typescript-bot
Copy link
Collaborator

typescript-bot commented Apr 25, 2022

Heya @ahejlsberg, I've started to run the parallelized Definitely Typed test suite on this PR at 2a7fc1e. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Apr 25, 2022

Heya @ahejlsberg, I've started to run the extended test suite on this PR at 2a7fc1e. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Apr 25, 2022

Heya @ahejlsberg, I've started to run the abridged perf test suite on this PR at 2a7fc1e. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Apr 25, 2022

Heya @ahejlsberg, I've started to run the diff-based community code test suite on this PR at 2a7fc1e. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

@ahejlsberg
The results of the perf run you requested are in!

Here they are:

Comparison Report - main..48837

Metric main 48837 Delta Best Worst
Angular - node (v14.15.1, x64)
Memory used 333,395k (± 0.01%) 333,407k (± 0.01%) +12k (+ 0.00%) 333,334k 333,472k
Parse Time 2.04s (± 0.68%) 2.03s (± 0.63%) -0.01s (- 0.44%) 2.00s 2.05s
Bind Time 0.88s (± 1.07%) 0.87s (± 0.60%) -0.01s (- 0.91%) 0.86s 0.88s
Check Time 5.62s (± 0.44%) 5.63s (± 0.38%) +0.00s (+ 0.09%) 5.57s 5.68s
Emit Time 6.34s (± 0.59%) 6.31s (± 0.48%) -0.03s (- 0.52%) 6.24s 6.40s
Total Time 14.88s (± 0.27%) 14.84s (± 0.38%) -0.04s (- 0.30%) 14.72s 14.99s
Compiler-Unions - node (v14.15.1, x64)
Memory used 192,015k (± 0.12%) 192,182k (± 0.02%) +168k (+ 0.09%) 192,113k 192,265k
Parse Time 0.86s (± 0.69%) 0.85s (± 0.56%) -0.00s (- 0.35%) 0.84s 0.86s
Bind Time 0.56s (± 0.60%) 0.56s (± 0.84%) 0.00s ( 0.00%) 0.56s 0.58s
Check Time 7.48s (± 0.40%) 7.49s (± 0.60%) +0.00s (+ 0.05%) 7.39s 7.57s
Emit Time 2.53s (± 1.26%) 2.51s (± 0.49%) -0.02s (- 0.75%) 2.49s 2.55s
Total Time 11.43s (± 0.54%) 11.41s (± 0.38%) -0.02s (- 0.15%) 11.32s 11.50s
Monaco - node (v14.15.1, x64)
Memory used 325,465k (± 0.01%) 325,496k (± 0.00%) +32k (+ 0.01%) 325,462k 325,533k
Parse Time 1.58s (± 0.77%) 1.57s (± 0.56%) -0.01s (- 0.32%) 1.55s 1.59s
Bind Time 0.77s (± 0.47%) 0.78s (± 1.03%) +0.00s (+ 0.52%) 0.77s 0.80s
Check Time 5.56s (± 0.32%) 5.54s (± 0.56%) -0.02s (- 0.36%) 5.48s 5.58s
Emit Time 3.34s (± 0.68%) 3.31s (± 0.55%) -0.03s (- 0.75%) 3.28s 3.37s
Total Time 11.25s (± 0.29%) 11.21s (± 0.37%) -0.04s (- 0.37%) 11.11s 11.29s
TFS - node (v14.15.1, x64)
Memory used 289,043k (± 0.01%) 289,062k (± 0.01%) +19k (+ 0.01%) 289,010k 289,141k
Parse Time 1.36s (± 0.69%) 1.36s (± 0.91%) +0.00s (+ 0.00%) 1.34s 1.39s
Bind Time 0.72s (± 0.65%) 0.72s (± 0.50%) +0.00s (+ 0.56%) 0.72s 0.73s
Check Time 5.20s (± 0.47%) 5.19s (± 0.46%) -0.01s (- 0.12%) 5.16s 5.26s
Emit Time 3.57s (± 1.76%) 3.51s (± 2.22%) -0.06s (- 1.74%) 3.38s 3.63s
Total Time 10.85s (± 0.71%) 10.78s (± 0.81%) -0.07s (- 0.65%) 10.62s 10.95s
material-ui - node (v14.15.1, x64)
Memory used 447,810k (± 0.01%) 447,477k (± 0.08%) -333k (- 0.07%) 446,630k 447,844k
Parse Time 1.87s (± 0.64%) 1.87s (± 0.39%) -0.01s (- 0.27%) 1.86s 1.89s
Bind Time 0.69s (± 0.69%) 0.70s (± 1.07%) +0.00s (+ 0.58%) 0.69s 0.72s
Check Time 13.03s (± 0.62%) 13.12s (± 0.78%) +0.10s (+ 0.75%) 12.81s 13.31s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 15.59s (± 0.54%) 15.69s (± 0.65%) +0.09s (+ 0.58%) 15.39s 15.88s
xstate - node (v14.15.1, x64)
Memory used 535,805k (± 0.00%) 535,897k (± 0.00%) +92k (+ 0.02%) 535,860k 535,929k
Parse Time 2.60s (± 0.48%) 2.60s (± 0.49%) +0.00s (+ 0.15%) 2.57s 2.62s
Bind Time 1.15s (± 0.65%) 1.14s (± 0.98%) -0.01s (- 0.78%) 1.13s 1.18s
Check Time 1.51s (± 0.62%) 1.51s (± 0.72%) +0.00s (+ 0.27%) 1.50s 1.54s
Emit Time 0.07s (± 0.00%) 0.07s (± 0.00%) 0.00s ( 0.00%) 0.07s 0.07s
Total Time 5.33s (± 0.39%) 5.33s (± 0.59%) -0.00s (- 0.02%) 5.28s 5.41s
System
Machine Namets-ci-ubuntu
Platformlinux 4.4.0-210-generic
Architecturex64
Available Memory16 GB
Available Memory2 GB
CPUs4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz
Hosts
  • node (v14.15.1, x64)
Scenarios
  • Angular - node (v14.15.1, x64)
  • Compiler-Unions - node (v14.15.1, x64)
  • Monaco - node (v14.15.1, x64)
  • TFS - node (v14.15.1, x64)
  • material-ui - node (v14.15.1, x64)
  • xstate - node (v14.15.1, x64)
Benchmark Name Iterations
Current 48837 10
Baseline main 10

Developer Information:

Download Benchmark

@typescript-bot
Copy link
Collaborator

@ahejlsberg
Great news! no new errors were found between main..refs/pull/48837/merge

@Andarist
Copy link
Contributor

I believe that this will close #48739

@ahejlsberg
Copy link
Member Author

Tests and perf all look to be unaffected.

@jcalz
Copy link
Contributor

jcalz commented Apr 26, 2022

Could someone revisit #41893 and explain what's going on here and how it either meshes with or supersedes it? The argument there that is that array["-0.0e123"] is not going to be the same as array[0] and so `${number}` cannot be used to into numeric index signatures because there's no requirement that the numeric strings in question round trip back to themselves:

const arr: string[] = [];
const x: `${number}` = "-0.0e123";
arr[x] = "oopsie"; // <-- is this supposed to be okay or an error?

Personally I'd love it if such a requirement existed and that `${number}` would reject "-0.0e123" because the plain reading of `${number}` is "what you get if you serialize a number via a template literal string". And it agrees with what happens if you put a numeric literal or union of literals instead of number there. `${0}` is not "any string which yields 0 when coerced to a number", it's just "0". I had given up all hope of this happening, because democracy.

But but now if you're doing this, it means that you agree that arrays and tuples have keys of type `${number}`, which means that there's a chance of revisiting this and making everything consistent, right? R...right? Sigh. Assuming that this will not change, then can we hear some authoritative words about it so I can at least parrot them back when people ask what's going on with this? (Edit: seems like there's a disconnect between the discussion in #48599 and the ones in #48739 and #41893, so I'd love to have that discrepancy cleared up.)

@ahejlsberg
Copy link
Member Author

Could someone revisit #41893 and explain what's going on here and how it either meshes with or supersedes it?

Yes, see here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants