Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Support interpreting non-literal computed properties in classes as implicit index signatures#59860

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

weswigham
Copy link
Member

Instead of silently dropping them (if they're declared by methods) as we do today.

This still has some minor issues and open questions, but is far enough along to share and talk about. Today, when you have a class like

declareconsta:symbol;exportclassA{[a](){return1};}

that's just fine - no errors anywhere. But if you add a

declareconste1:A[typeofa];

you get an error thatA has no symbol index signature. What's up with that? Well, we dropped the[a] member entirely, silently. There are historical reasons for this, but they don't really hold much weight nowadays beyond "for back compat". Under non-strict, this can actually be insidious, as when you use an expression, the(new A())[a] expression can silently beany, since under non-strict mode, non-existing element accesses resolve toany without error. With this PR,A[typeof a] instead resolves to() => number (the method's type) as we've created an implicit index signature for the computed name, just like we do for object literals.

Technically, this involves late-binding the__index member of the class like we do other computed literal names (so you could call these "late bound index signatures" as I do in the code), but that's not so important compared to the effect. This is pretty different from how these kinds of indexes are built for object literals, but that's because object literals are built somewhat on-demand from their constituent declarations, rather than having a big table of declared metadata built up-front like classes and interfaces do.

The big questions I'm still considering are:

  • Should all the indexes for the same key types get combined? Right now for objects they do, but in the implementation, the first one decides the signature type and errors are issued on any others that don't match. (Is our behavior for objects good?)
  • Should the error on non-literal computed properties,A computed property name in a class property declaration must have a simple literal type or a 'unique symbol' type.(1166) be removed, too? Methods don't emit an error, which is what motivated me here, but all this logic equally applies to properties, so the error doesn't make too much sense for them now.
  • Should this require opt-in vianoImplicitAny or another (strict?) flag? (preferably not - this being flagged somewhat defeats the point, imo)

And, ofc, this needs more tests, covering other key types (unions, pattern literals) and modifiers (static, readonly) on the computed names (though I've informally handled them in the code, I think) before this can actually be merged (though these probably have some coverage already from documenting our current behavior - they could stand to be more comprehensive).

This is, IMO, a worthwhile step towards unifying all of our computed property name behaviors. In an ideal world, a computed property name in a class, object, in a source or declaration file, could all be interpreted the same way (and thus, it could always be safe to copy one from one place or file kind into the other). Unfortunately, today we have some behavioral carve-outs from the time before we even meaningfully supported literal types, or even had--strict, that get in the way of that, like this.

Didas-git and tonivj5 reacted with heart emoji
@weswigham
Copy link
MemberAuthor

@typescript-bot pack this

typescript-bot reacted with thumbs up emoji

@typescript-bot
Copy link
Collaborator

typescript-bot commentedSep 4, 2024•
edited
Loading

Starting jobs; this comment will be updated as builds start and complete.

CommandStatusResults
pack this✅ Started✅ Results

@typescript-bot
Copy link
Collaborator

typescript-bot commentedSep 4, 2024•
edited
Loading

Hey@weswigham, I've packed this intoan installable tgz. You can install it for testing by referencing it in yourpackage.json like so:

{    "devDependencies": {        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/163508/artifacts?artifactName=tgz&fileId=BC8EAA3AB4A7092D73B64AB5D33BC556C27F8482F31A3222954DD7E6A6A4573002&fileName=/typescript-5.7.0-insiders.20240904.tgz"    }}

and then runningnpm install.

There is also a playgroundfor this build and annpm module you can use via"typescript": "npm:@typescript-deploys/pr-build@5.7.0-pr-59860-2".;

@weswigham
Copy link
MemberAuthor

@typescript-bot test it

typescript-bot reacted with thumbs up emoji

@typescript-bot
Copy link
Collaborator

typescript-bot commentedSep 6, 2024•
edited
Loading

Starting jobs; this comment will be updated as builds start and complete.

CommandStatusResults
test top400✅ Started👀 Results
user test this✅ Started✅ Results
run dt✅ Started✅ Results
perf test this faster✅ Started👀 Results

@typescript-bot
Copy link
Collaborator

Hey@weswigham, the results of running the DT tests are ready.

Everything looks the same!

You can check the log here.

@typescript-bot
Copy link
Collaborator

@weswigham Here are the results of running the user tests with tsc comparingmain andrefs/pull/59860/merge:

Everything looks good!

@typescript-bot
Copy link
Collaborator

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

Here they are:

tsc

Comparison Report - baseline..pr
MetricbaselineprDeltaBestWorstp-value
Compiler-Unions - node (v18.15.0, x64)
Errors3030~~~p=1.000 n=6
Symbols62,15362,153~~~p=1.000 n=6
Types50,24250,242~~~p=1.000 n=6
Memory used194,194k (± 0.98%)193,647k (± 0.98%)~192,379k196,219kp=0.689 n=6
Parse Time1.31s (± 0.31%)1.31s (± 0.75%)~1.29s1.32sp=0.753 n=6
Bind Time0.71s0.71s~~~p=1.000 n=6
Check Time9.56s (± 0.48%)9.58s (± 0.25%)~9.54s9.61sp=0.517 n=6
Emit Time2.74s (± 0.60%)2.72s (± 0.63%)~2.70s2.75sp=0.164 n=6
Total Time14.31s (± 0.45%)14.32s (± 0.23%)~14.27s14.36sp=1.000 n=6
angular-1 - node (v18.15.0, x64)
Errors711🔻+4 (+57.14%)~~p=0.001 n=6
Symbols945,753945,755+2 (+ 0.00%)~~p=0.001 n=6
Types410,067410,072+5 (+ 0.00%)~~p=0.001 n=6
Memory used1,222,741k (± 0.00%)1,222,807k (± 0.00%)+65k (+ 0.01%)1,222,775k1,222,872kp=0.005 n=6
Parse Time6.63s (± 0.70%)6.65s (± 0.39%)~6.62s6.69sp=0.572 n=6
Bind Time1.86s (± 0.40%)1.86s (± 0.28%)~1.86s1.87sp=0.784 n=6
Check Time31.17s (± 0.25%)31.31s (± 0.43%)~31.11s31.49sp=0.078 n=6
Emit Time15.06s (± 0.16%)15.04s (± 0.44%)~14.97s15.15sp=0.519 n=6
Total Time54.73s (± 0.18%)54.87s (± 0.12%)+0.14s (+ 0.25%)54.79s54.97sp=0.020 n=6
mui-docs - node (v18.15.0, x64)
Errors00~~~p=1.000 n=6
Symbols2,505,9622,505,962~~~p=1.000 n=6
Types932,873932,873~~~p=1.000 n=6
Memory used2,334,309k (± 0.00%)2,334,335k (± 0.00%)~2,334,247k2,334,488kp=0.336 n=6
Parse Time9.24s (± 0.16%)9.26s (± 0.21%)~9.24s9.29sp=0.162 n=6
Bind Time2.17s (± 0.61%)2.16s (± 0.91%)~2.12s2.17sp=0.145 n=6
Check Time72.58s (± 0.35%)72.60s (± 0.34%)~72.27s72.88sp=0.936 n=6
Emit Time0.28s (± 2.95%)0.28s (± 1.86%)~0.27s0.28sp=0.752 n=6
Total Time84.27s (± 0.29%)84.30s (± 0.30%)~83.97s84.57sp=0.936 n=6
self-build-src - node (v18.15.0, x64)
Errors00~~~p=1.000 n=6
Symbols1,247,0481,247,344+296 (+ 0.02%)~~p=0.001 n=6
Types265,107265,257+150 (+ 0.06%)~~p=0.001 n=6
Memory used2,398,167k (± 0.01%)2,398,870k (± 0.01%)+703k (+ 0.03%)2,398,532k2,399,143kp=0.005 n=6
Parse Time5.03s (± 0.49%)5.01s (± 0.81%)~4.94s5.05sp=0.809 n=6
Bind Time1.94s (± 0.53%)1.93s (± 0.60%)~1.92s1.95sp=0.456 n=6
Check Time34.91s (± 0.34%)34.90s (± 0.21%)~34.83s35.03sp=0.936 n=6
Emit Time3.31s (± 1.33%)3.30s (± 0.78%)~3.27s3.33sp=0.572 n=6
Total Time45.19s (± 0.26%)45.15s (± 0.18%)~45.06s45.26sp=0.810 n=6
self-build-src-public-api - node (v18.15.0, x64)
Errors00~~~p=1.000 n=6
Symbols1,247,0481,247,344+296 (+ 0.02%)~~p=0.001 n=6
Types265,107265,257+150 (+ 0.06%)~~p=0.001 n=6
Memory used2,472,724k (± 0.03%)2,473,350k (± 0.02%)~2,472,808k2,473,969kp=0.093 n=6
Parse Time7.85s (± 0.78%)7.82s (± 0.60%)~7.78s7.90sp=0.471 n=6
Bind Time2.56s (± 0.48%)2.54s (± 0.43%)~2.53s2.56sp=0.096 n=6
Check Time50.95s (± 0.35%)51.20s (± 0.62%)~50.91s51.76sp=0.128 n=6
Emit Time4.92s (± 1.10%)4.96s (± 1.80%)~4.86s5.12sp=0.471 n=6
Total Time66.28s (± 0.32%)66.53s (± 0.46%)~66.17s67.06sp=0.229 n=6
self-compiler - node (v18.15.0, x64)
Errors00~~~p=1.000 n=6
Symbols259,129259,359+230 (+ 0.09%)~~p=0.001 n=6
Types105,985106,135+150 (+ 0.14%)~~p=0.001 n=6
Memory used433,699k (± 0.05%)433,874k (± 0.03%)~433,741k434,072kp=0.128 n=6
Parse Time3.42s (± 0.71%)3.41s (± 0.63%)~3.38s3.44sp=0.332 n=6
Bind Time1.27s (± 0.66%)1.28s (± 1.10%)~1.26s1.30sp=0.503 n=6
Check Time18.12s (± 0.24%)18.06s (± 0.19%)~18.01s18.10sp=0.054 n=6
Emit Time1.68s (± 1.11%)1.67s (± 1.40%)~1.64s1.70sp=0.871 n=6
Total Time24.50s (± 0.18%)24.43s (± 0.11%)-0.07s (- 0.30%)24.39s24.46sp=0.010 n=6
ts-pre-modules - node (v18.15.0, x64)
Errors6868~~~p=1.000 n=6
Symbols225,018225,018~~~p=1.000 n=6
Types94,24994,249~~~p=1.000 n=6
Memory used370,246k (± 0.02%)370,316k (± 0.02%)~370,217k370,437kp=0.173 n=6
Parse Time2.77s (± 0.99%)2.75s (± 0.78%)~2.73s2.79sp=0.157 n=6
Bind Time1.57s (± 1.25%)1.57s (± 0.74%)~1.56s1.59sp=0.863 n=6
Check Time15.78s (± 0.45%)15.78s (± 0.63%)~15.60s15.88sp=1.000 n=6
Emit Time0.00s0.00s~~~p=1.000 n=6
Total Time20.13s (± 0.46%)20.10s (± 0.43%)~19.96s20.20sp=0.809 n=6
vscode - node (v18.15.0, x64)
Errors11~~~p=1.000 n=6
Symbols3,041,3363,041,337+1 (+ 0.00%)~~p=0.001 n=6
Types1,051,6171,051,617~~~p=1.000 n=6
Memory used3,149,210k (± 0.00%)3,149,235k (± 0.00%)~3,149,071k3,149,345kp=0.689 n=6
Parse Time13.80s (± 0.35%)13.81s (± 0.58%)~13.69s13.93sp=0.872 n=6
Bind Time4.42s (± 2.41%)4.43s (± 2.81%)~4.31s4.58sp=1.000 n=6
Check Time81.18s (± 0.27%)81.49s (± 0.46%)~80.75s81.77sp=0.065 n=6
Emit Time22.10s (± 0.71%)22.00s (± 0.56%)~21.81s22.17sp=0.471 n=6
Total Time121.50s (± 0.28%)121.73s (± 0.46%)~120.65s122.12sp=0.229 n=6
webpack - node (v18.15.0, x64)
Errors00~~~p=1.000 n=6
Symbols277,017277,018+1 (+ 0.00%)~~p=0.001 n=6
Types112,816112,816~~~p=1.000 n=6
Memory used426,811k (± 0.03%)426,779k (± 0.02%)~426,665k426,882kp=0.810 n=6
Parse Time4.90s (± 0.42%)4.88s (± 0.31%)~4.87s4.91sp=0.256 n=6
Bind Time2.14s (± 0.82%)2.15s (± 1.44%)~2.10s2.18sp=1.000 n=6
Check Time21.79s (± 0.48%)21.70s (± 0.43%)~21.57s21.83sp=0.149 n=6
Emit Time0.00s0.00s~~~p=1.000 n=6
Total Time28.83s (± 0.37%)28.73s (± 0.26%)~28.63s28.81sp=0.106 n=6
xstate-main - node (v18.15.0, x64)
Errors00~~~p=1.000 n=6
Symbols539,317539,317~~~p=1.000 n=6
Types178,997178,997~~~p=1.000 n=6
Memory used484,147k (± 0.03%)484,064k (± 0.03%)~483,872k484,249kp=0.378 n=6
Parse Time4.26s (± 0.39%)4.24s (± 0.32%)-0.02s (- 0.55%)4.22s4.26sp=0.033 n=6
Bind Time1.55s (± 1.05%)1.55s (± 0.75%)~1.54s1.57sp=1.000 n=6
Check Time22.51s (± 0.33%)22.56s (± 0.45%)~22.43s22.65sp=0.336 n=6
Emit Time0.00s0.00s~~~p=1.000 n=6
Total Time28.33s (± 0.29%)28.35s (± 0.34%)~28.22s28.44sp=0.809 n=6
System info unknown
Hosts
  • node (v18.15.0, x64)
Scenarios
  • Compiler-Unions - node (v18.15.0, x64)
  • angular-1 - node (v18.15.0, x64)
  • mui-docs - node (v18.15.0, x64)
  • self-build-src - node (v18.15.0, x64)
  • self-build-src-public-api - node (v18.15.0, x64)
  • self-compiler - node (v18.15.0, x64)
  • ts-pre-modules - node (v18.15.0, x64)
  • vscode - node (v18.15.0, x64)
  • webpack - node (v18.15.0, x64)
  • xstate-main - node (v18.15.0, x64)
BenchmarkNameIterations
Currentpr6
Baselinebaseline6

Developer Information:

Download Benchmarks

@jakebailey
Copy link
Member

Seems like the angular codebase is gaining new errors?

@typescript-bot
Copy link
Collaborator

@weswigham Here are the results of running the top 400 repos with tsc comparingmain andrefs/pull/59860/merge:

Something interesting changed - please have a look.

Details

jhipster/generator-jhipster

1 of 2 projects failed to build with the old tsc and were ignored

tsconfig.json

  • error TS2411: Property 'baseName' of type 'string' is not assignable to 'string' index type 'GenericTaskGroup<any, TaskParamWithControl, "convertToJDL">'.
  • error TS2411: Property 'jdlFile' of type 'string' is not assignable to 'string' index type 'GenericTaskGroup<any, TaskParamWithControl, "convertToJDL">'.
  • error TS2411: Property 'jdlContent' of type 'string | undefined' is not assignable to 'string' index type 'GenericTaskGroup<any, TaskParamWithControl, "convertToJDL">'.
  • error TS2411: Property '[BaseGenerator.WRITING]' of type 'GenericTaskGroup<any, TaskParamWithControl, "writeJdl">' is not assignable to 'string' index type 'GenericTaskGroup<any, TaskParamWithControl, "convertToJDL">'.
  • error TS2411: Property '[BaseGenerator.END]' of type 'GenericTaskGroup<any, TaskParamWithControl, "end">' is not assignable to 'string' index type 'GenericTaskGroup<any, TaskParamWithControl, "convertToJDL">'.
  • error TS2411: Property 'checkCommand' of type '(command: string, args: string[], printInfo?: (result: { stdout: string; stderr: string; }) => void) => Promise<void>' is not assignable to 'string' index type 'GenericTaskGroup<any, TaskParamWithControl, "checkGit" | "sayHello" | "checkJHipster" | "displayConfiguration" | "checkJava" | "checkNode" | "checkNpm" | "checkDocker" | "checkApplication" | "displayEntities">'.
  • error TS2411: Property 'generateJDLFromEntities' of type '() => any' is not assignable to 'string' index type 'GenericTaskGroup<any, TaskParamWithControl, "checkGit" | "sayHello" | "checkJHipster" | "displayConfiguration" | "checkJava" | "checkNode" | "checkNpm" | "checkDocker" | "checkApplication" | "displayEntities">'.
  • error TS2411: Property 'beforeQueue' of type '() => Promise<void>' is not assignable to 'string' index type 'GenericTaskGroup<any, TaskParamWithControl, "askForServerTestOpts" | "askForServerSideOpts" | "askForOptionalItems">'.
  • error TS2411: Property 'writingEntities' of type 'GenericTaskGroup<any, TaskParamWithEntities<Entity<Field, never>, ApplicationType<Entity<Field, never>>>, "writeServerFiles" | "cleanupOldServerFiles">' is not assignable to 'string' index type 'GenericTaskGroup<any, TaskParamWithControl, "askForServerTestOpts" | "askForServerSideOpts" | "askForOptionalItems">'.
  • error TS2411: Property '[BaseApplicationGenerator.WRITING_ENTITIES]' of type 'GenericTaskGroup<any, TaskParamWithEntities<Entity<Field, never>, ApplicationType<Entity<Field, never>>>, "writeServerFiles" | "cleanupOldServerFiles">' is not assignable to 'string' index type 'GenericTaskGroup<any, TaskParamWithControl, "askForServerTestOpts" | "askForServerSideOpts" | "askForOptionalItems">'.
  • error TS2411: Property 'getPrimaryKeyValue' of type '(primaryKey: any, databaseType?: any, defaultValue?: number) => string' is not assignable to 'string' index type 'GenericTaskGroup<any, TaskParamWithControl, "askForServerTestOpts" | "askForServerSideOpts" | "askForOptionalItems">'.

@weswigham
Copy link
MemberAuthor

@typescript-bot test it

Now I'm usingexactly the same index signature creation logic we use for object literal expressions, with all of it's flaws (like making a lot ofnumber index signatures you may not expect). Let's see how this goes.

typescript-bot reacted with thumbs up emoji

@typescript-bot
Copy link
Collaborator

typescript-bot commentedSep 9, 2024•
edited
Loading

Starting jobs; this comment will be updated as builds start and complete.

CommandStatusResults
test top400✅ Started👀 Results
user test this✅ Started✅ Results
run dt✅ Started✅ Results
perf test this faster✅ Started👀 Results

@typescript-bot
Copy link
Collaborator

Hey@weswigham, the results of running the DT tests are ready.

Everything looks the same!

You can check the log here.

@typescript-bot
Copy link
Collaborator

@weswigham Here are the results of running the user tests with tsc comparingmain andrefs/pull/59860/merge:

Everything looks good!

@typescript-bot
Copy link
Collaborator

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

Here they are:

tsc

Comparison Report - baseline..pr
MetricbaselineprDeltaBestWorstp-value
Compiler-Unions - node (v18.15.0, x64)
Errors3030~~~p=1.000 n=6
Symbols62,15362,153~~~p=1.000 n=6
Types50,24250,242~~~p=1.000 n=6
Memory used193,574k (± 0.93%)193,071k (± 0.73%)~192,407k195,949kp=0.936 n=6
Parse Time1.58s (± 0.93%)1.57s (± 0.87%)~1.55s1.59sp=0.510 n=6
Bind Time0.86s (± 1.04%)0.86s (± 1.27%)~0.85s0.87sp=1.000 n=6
Check Time11.36s (± 0.47%)11.40s (± 0.38%)~11.35s11.47sp=0.090 n=6
Emit Time3.25s (± 0.75%)3.24s (± 0.95%)~3.20s3.28sp=0.516 n=6
Total Time17.04s (± 0.42%)17.07s (± 0.47%)~16.96s17.17sp=0.686 n=6
angular-1 - node (v18.15.0, x64)
Errors711🔻+4 (+57.14%)~~p=0.001 n=6
Symbols945,753945,762+9 (+ 0.00%)~~p=0.001 n=6
Types410,067410,075+8 (+ 0.00%)~~p=0.001 n=6
Memory used1,222,717k (± 0.00%)1,222,759k (± 0.00%)~1,222,662k1,222,829kp=0.173 n=6
Parse Time7.92s (± 0.53%)7.93s (± 0.31%)~7.90s7.95sp=0.744 n=6
Bind Time2.23s (± 0.23%)2.23s (± 0.38%)~2.22s2.24sp=0.533 n=6
Check Time36.35s (± 0.41%)36.56s (± 0.44%)~36.39s36.77sp=0.054 n=6
Emit Time17.88s (± 0.53%)17.86s (± 0.22%)~17.79s17.91sp=0.419 n=6
Total Time64.38s (± 0.30%)64.57s (± 0.26%)~64.37s64.74sp=0.127 n=6
mui-docs - node (v18.15.0, x64)
Errors00~~~p=1.000 n=6
Symbols2,506,9322,506,986+54 (+ 0.00%)~~p=0.001 n=6
Types932,998933,017+19 (+ 0.00%)~~p=0.001 n=6
Memory used2,336,489k (± 0.00%)2,336,552k (± 0.00%)~2,336,495k2,336,618kp=0.173 n=6
Parse Time11.01s (± 0.29%)11.01s (± 0.28%)~10.95s11.04sp=1.000 n=6
Bind Time2.57s (± 0.94%)2.57s (± 0.55%)~2.55s2.59sp=0.745 n=6
Check Time85.98s (± 0.32%)86.17s (± 0.39%)~85.62s86.50sp=0.336 n=6
Emit Time0.32s (± 2.34%)0.34s (± 1.53%)🔻+0.01s (+ 4.66%)0.33s0.34sp=0.010 n=6
Total Time99.89s (± 0.29%)100.09s (± 0.32%)~99.55s100.41sp=0.230 n=6
self-build-src - node (v18.15.0, x64)
Errors00~~~p=1.000 n=6
Symbols1,250,2131,250,555+342 (+ 0.03%)~~p=0.001 n=6
Types265,146265,304+158 (+ 0.06%)~~p=0.001 n=6
Memory used2,402,574k (± 0.02%)2,403,217k (± 0.03%)~2,402,219k2,403,761kp=0.128 n=6
Parse Time6.09s (± 0.79%)6.11s (± 0.61%)~6.08s6.17sp=0.689 n=6
Bind Time2.26s (± 0.81%)2.29s (± 1.48%)~2.25s2.33sp=0.125 n=6
Check Time41.23s (± 1.07%)41.10s (± 0.57%)~40.80s41.35sp=0.378 n=6
Emit Time3.97s (± 1.11%)4.00s (± 3.73%)~3.88s4.29sp=0.689 n=6
Total Time53.57s (± 0.90%)53.50s (± 0.51%)~53.11s53.79sp=0.575 n=6
self-build-src-public-api - node (v18.15.0, x64)
Errors00~~~p=1.000 n=6
Symbols1,250,2131,250,555+342 (+ 0.03%)~~p=0.001 n=6
Types265,146265,304+158 (+ 0.06%)~~p=0.001 n=6
Memory used2,477,482k (± 0.02%)2,477,718k (± 0.02%)~2,477,053k2,478,317kp=0.378 n=6
Parse Time7.82s (± 0.98%)7.84s (± 0.65%)~7.79s7.92sp=0.810 n=6
Bind Time2.52s (± 0.54%)2.54s (± 0.75%)~2.52s2.57sp=0.197 n=6
Check Time51.24s (± 0.77%)51.13s (± 0.72%)~50.78s51.78sp=0.471 n=6
Emit Time4.96s (± 0.77%)5.03s (± 3.72%)~4.89s5.40sp=0.810 n=6
Total Time66.54s (± 0.53%)66.57s (± 0.56%)~66.21s67.24sp=0.873 n=6
self-compiler - node (v18.15.0, x64)
Errors00~~~p=1.000 n=6
Symbols259,449259,685+236 (+ 0.09%)~~p=0.001 n=6
Types105,986106,135+149 (+ 0.14%)~~p=0.001 n=6
Memory used433,838k (± 0.02%)434,199k (± 0.02%)+360k (+ 0.08%)434,085k434,366kp=0.005 n=6
Parse Time4.21s (± 0.84%)4.24s (± 1.04%)~4.16s4.29sp=0.293 n=6
Bind Time1.57s (± 1.31%)1.57s (± 0.70%)~1.56s1.59sp=0.933 n=6
Check Time22.36s (± 0.25%)22.38s (± 0.33%)~22.24s22.46sp=0.418 n=6
Emit Time2.04s (± 1.14%)2.02s (± 0.99%)-0.02s (- 1.14%)1.98s2.03sp=0.040 n=6
Total Time30.20s (± 0.19%)30.21s (± 0.35%)~30.02s30.32sp=0.470 n=6
ts-pre-modules - node (v18.15.0, x64)
Errors6868~~~p=1.000 n=6
Symbols225,018225,018~~~p=1.000 n=6
Types94,24994,249~~~p=1.000 n=6
Memory used370,218k (± 0.02%)370,252k (± 0.02%)~370,176k370,333kp=0.229 n=6
Parse Time3.45s (± 1.07%)3.42s (± 0.55%)~3.39s3.44sp=0.124 n=6
Bind Time1.92s (± 0.51%)1.93s (± 0.87%)~1.91s1.96sp=0.315 n=6
Check Time19.44s (± 0.27%)19.46s (± 0.51%)~19.33s19.60sp=0.630 n=6
Emit Time0.00s0.00s~~~p=1.000 n=6
Total Time24.81s (± 0.26%)24.81s (± 0.38%)~24.70s24.95sp=0.810 n=6
vscode - node (v18.15.0, x64)
Errors11~~~p=1.000 n=6
Symbols3,041,3193,041,378+59 (+ 0.00%)~~p=0.001 n=6
Types1,051,7971,051,818+21 (+ 0.00%)~~p=0.001 n=6
Memory used3,149,846k (± 0.00%)3,149,913k (± 0.00%)+68k (+ 0.00%)3,149,883k3,149,982kp=0.030 n=6
Parse Time13.77s (± 0.28%)13.84s (± 0.52%)~13.75s13.94sp=0.090 n=6
Bind Time4.45s (± 2.63%)4.49s (± 2.40%)~4.35s4.58sp=0.328 n=6
Check Time81.60s (± 0.33%)81.63s (± 0.49%)~81.02s82.00sp=1.000 n=6
Emit Time22.11s (± 0.37%)22.03s (± 0.87%)~21.87s22.39sp=0.173 n=6
Total Time121.92s (± 0.23%)121.99s (± 0.38%)~121.41s122.56sp=0.689 n=6
webpack - node (v18.15.0, x64)
Errors00~~~p=1.000 n=6
Symbols277,022276,955-67 (- 0.02%)~~p=0.001 n=6
Types112,817112,973+156 (+ 0.14%)~~p=0.001 n=6
Memory used426,673k (± 0.02%)426,816k (± 0.02%)+143k (+ 0.03%)426,713k426,917kp=0.020 n=6
Parse Time3.95s (± 0.44%)3.95s (± 0.41%)~3.93s3.97sp=0.869 n=6
Bind Time1.71s (± 0.24%)1.71s (± 1.01%)~1.68s1.73sp=0.528 n=6
Check Time17.59s (± 0.39%)17.58s (± 0.52%)~17.47s17.69sp=0.574 n=6
Emit Time0.00s0.00s~~~p=1.000 n=6
Total Time23.26s (± 0.26%)23.24s (± 0.49%)~23.11s23.38sp=0.689 n=6
xstate-main - node (v18.15.0, x64)
Errors00~~~p=1.000 n=6
Symbols539,317539,355+38 (+ 0.01%)~~p=0.001 n=6
Types178,997179,004+7 (+ 0.00%)~~p=0.001 n=6
Memory used484,293k (± 0.02%)484,268k (± 0.04%)~483,947k484,472kp=0.936 n=6
Parse Time2.84s (± 0.56%)2.84s (± 0.44%)~2.82s2.85sp=0.869 n=6
Bind Time1.05s (± 0.78%)1.05s~~~p=0.405 n=6
Check Time15.49s (± 0.41%)15.76s (± 0.28%)+0.27s (+ 1.73%)15.71s15.82sp=0.005 n=6
Emit Time0.00s0.00s~~~p=1.000 n=6
Total Time19.37s (± 0.34%)19.64s (± 0.21%)+0.27s (+ 1.39%)19.60s19.70sp=0.005 n=6
System info unknown
Hosts
  • node (v18.15.0, x64)
Scenarios
  • Compiler-Unions - node (v18.15.0, x64)
  • angular-1 - node (v18.15.0, x64)
  • mui-docs - node (v18.15.0, x64)
  • self-build-src - node (v18.15.0, x64)
  • self-build-src-public-api - node (v18.15.0, x64)
  • self-compiler - node (v18.15.0, x64)
  • ts-pre-modules - node (v18.15.0, x64)
  • vscode - node (v18.15.0, x64)
  • webpack - node (v18.15.0, x64)
  • xstate-main - node (v18.15.0, x64)
BenchmarkNameIterations
Currentpr6
Baselinebaseline6

Developer Information:

Download Benchmarks

@weswigham
Copy link
MemberAuthor

Hm. Still some new errors in angular, but everything else looks fine now. Guess I'll have to look at what those are.

@typescript-bot
Copy link
Collaborator

@weswigham Here are the results of running the top 400 repos with tsc comparingmain andrefs/pull/59860/merge:

Something interesting changed - please have a look.

Details

jhipster/generator-jhipster

1 of 2 projects failed to build with the old tsc and were ignored

tsconfig.json

  • error TS2411: Property 'baseName' of type 'string' is not assignable to 'string' index type 'GenericTaskGroup<any, TaskParamWithControl, "convertToJDL"> | GenericTaskGroup<any, TaskParamWithControl, "writeJdl"> | GenericTaskGroup<...>'.
  • error TS2411: Property 'jdlFile' of type 'string' is not assignable to 'string' index type 'GenericTaskGroup<any, TaskParamWithControl, "convertToJDL"> | GenericTaskGroup<any, TaskParamWithControl, "writeJdl"> | GenericTaskGroup<...>'.
  • error TS2411: Property 'jdlContent' of type 'string | undefined' is not assignable to 'string' index type 'GenericTaskGroup<any, TaskParamWithControl, "convertToJDL"> | GenericTaskGroup<any, TaskParamWithControl, "writeJdl"> | GenericTaskGroup<...>'.
  • error TS2411: Property 'checkCommand' of type '(command: string, args: string[], printInfo?: (result: { stdout: string; stderr: string; }) => void) => Promise<void>' is not assignable to 'string' index type 'GenericTaskGroup<any, TaskParamWithControl, "checkGit" | "sayHello" | "checkJHipster" | "displayConfiguration" | "checkJava" | "checkNode" | "checkNpm" | "checkDocker" | "checkApplication" | "displayEntities">'.
  • error TS2411: Property 'generateJDLFromEntities' of type '() => any' is not assignable to 'string' index type 'GenericTaskGroup<any, TaskParamWithControl, "checkGit" | "sayHello" | "checkJHipster" | "displayConfiguration" | "checkJava" | "checkNode" | "checkNpm" | "checkDocker" | "checkApplication" | "displayEntities">'.

@typescript-bot
Copy link
Collaborator

Looks like you're introducing a change to the public API surface area. If this includes breaking changes, please document themon our wiki's API Breaking Changes page.

Also, please make sure@DanielRosenwasser and@RyanCavanaugh are aware of the changes, just as a heads up.

@weswigham
Copy link
MemberAuthor

@typescript-bot test it

typescript-bot reacted with thumbs up emoji

@typescript-bot
Copy link
Collaborator

typescript-bot commentedSep 10, 2024•
edited
Loading

Starting jobs; this comment will be updated as builds start and complete.

CommandStatusResults
test top400✅ Started✅ Results
user test this✅ Started✅ Results
run dt✅ Started✅ Results
perf test this faster✅ Started👀 Results

@typescript-bot
Copy link
Collaborator

Hey@weswigham, the results of running the DT tests are ready.

Everything looks the same!

You can check the log here.

@typescript-bot
Copy link
Collaborator

@weswigham Here are the results of running the user tests with tsc comparingmain andrefs/pull/59860/merge:

Everything looks good!

@typescript-bot
Copy link
Collaborator

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

Here they are:

tsc

Comparison Report - baseline..pr
MetricbaselineprDeltaBestWorstp-value
Compiler-Unions - node (v18.15.0, x64)
Errors3030~~~p=1.000 n=6
Symbols62,15362,153~~~p=1.000 n=6
Types50,24250,242~~~p=1.000 n=6
Memory used193,052k (± 0.77%)193,051k (± 0.75%)~192,436k196,011kp=0.298 n=6
Parse Time1.30s (± 0.84%)1.31s (± 0.39%)~1.30s1.31sp=0.321 n=6
Bind Time0.71s0.71s~~~p=1.000 n=6
Check Time9.58s (± 0.55%)9.58s (± 0.27%)~9.55s9.62sp=0.936 n=6
Emit Time2.72s (± 0.49%)2.72s (± 0.95%)~2.69s2.76sp=0.935 n=6
Total Time14.31s (± 0.38%)14.31s (± 0.29%)~14.26s14.38sp=0.936 n=6
angular-1 - node (v18.15.0, x64)
Errors77~~~p=1.000 n=6
Symbols945,753945,758+5 (+ 0.00%)~~p=0.001 n=6
Types410,067410,073+6 (+ 0.00%)~~p=0.001 n=6
Memory used1,222,743k (± 0.00%)1,222,820k (± 0.00%)~1,222,788k1,222,838kp=0.066 n=6
Parse Time6.62s (± 0.31%)6.67s (± 0.34%)+0.05s (+ 0.78%)6.65s6.71sp=0.006 n=6
Bind Time1.87s (± 0.29%)1.86s (± 0.53%)~1.84s1.87sp=0.201 n=6
Check Time31.18s (± 0.33%)31.29s (± 0.52%)~31.10s31.52sp=0.261 n=6
Emit Time15.00s (± 0.77%)15.01s (± 0.64%)~14.83s15.11sp=0.936 n=6
Total Time54.67s (± 0.14%)54.82s (± 0.44%)~54.54s55.20sp=0.296 n=6
mui-docs - node (v18.15.0, x64)
Errors00~~~p=1.000 n=6
Symbols2,515,5502,515,604+54 (+ 0.00%)~~p=0.001 n=6
Types933,023933,042+19 (+ 0.00%)~~p=0.001 n=6
Memory used2,350,381k (± 0.01%)2,350,413k (± 0.00%)~2,350,332k2,350,529kp=0.298 n=6
Parse Time9.43s (± 0.30%)9.43s (± 0.28%)~9.39s9.46sp=0.936 n=6
Bind Time2.18s (± 0.34%)2.17s (± 0.38%)~2.16s2.18sp=0.120 n=6
Check Time73.11s (± 1.41%)73.06s (± 0.48%)~72.56s73.48sp=0.470 n=6
Emit Time0.28s0.28s (± 2.95%)~0.27s0.29sp=0.290 n=6
Total Time85.00s (± 1.25%)84.93s (± 0.40%)~84.49s85.38sp=0.230 n=6
self-build-src - node (v18.15.0, x64)
Errors00~~~p=1.000 n=6
Symbols1,250,2131,250,582+369 (+ 0.03%)~~p=0.001 n=6
Types265,146265,307+161 (+ 0.06%)~~p=0.001 n=6
Memory used2,427,967k (± 2.59%)2,403,188k (± 0.03%)~2,402,166k2,403,855kp=0.378 n=6
Parse Time6.10s (± 0.42%)6.15s (± 0.35%)+0.04s (+ 0.71%)6.12s6.18sp=0.030 n=6
Bind Time2.29s (± 1.54%)2.28s (± 0.45%)~2.27s2.30sp=0.935 n=6
Check Time41.21s (± 0.96%)41.12s (± 0.52%)~40.92s41.42sp=0.689 n=6
Emit Time4.08s (± 3.88%)4.04s (± 4.69%)~3.88s4.31sp=0.378 n=6
Total Time53.71s (± 0.56%)53.62s (± 0.46%)~53.29s53.95sp=0.471 n=6
self-build-src-public-api - node (v18.15.0, x64)
Errors00~~~p=1.000 n=6
Symbols1,250,2131,250,582+369 (+ 0.03%)~~p=0.001 n=6
Types265,146265,307+161 (+ 0.06%)~~p=0.001 n=6
Memory used2,478,285k (± 0.11%)2,477,902k (± 0.02%)~2,477,333k2,478,582kp=0.298 n=6
Parse Time5.28s (± 0.44%)5.25s (± 0.88%)~5.19s5.31sp=0.199 n=6
Bind Time1.70s (± 0.53%)1.71s (± 0.88%)~1.70s1.73sp=0.156 n=6
Check Time35.13s (± 0.36%)35.13s (± 0.27%)~34.97s35.23sp=0.630 n=6
Emit Time3.35s (± 1.58%)3.40s (± 4.18%)~3.32s3.68sp=0.936 n=6
Total Time45.46s (± 0.35%)45.50s (± 0.47%)~45.22s45.87sp=0.575 n=6
self-compiler - node (v18.15.0, x64)
Errors00~~~p=1.000 n=6
Symbols259,449259,701+252 (+ 0.10%)~~p=0.001 n=6
Types105,986106,138+152 (+ 0.14%)~~p=0.001 n=6
Memory used433,843k (± 0.01%)434,207k (± 0.02%)+365k (+ 0.08%)434,075k434,322kp=0.005 n=6
Parse Time3.43s (± 0.76%)3.43s (± 0.87%)~3.39s3.47sp=1.000 n=6
Bind Time1.28s (± 0.40%)1.27s (± 0.32%)-0.01s (- 0.65%)1.26s1.27sp=0.022 n=6
Check Time18.16s (± 0.35%)18.09s (± 0.22%)~18.04s18.16sp=0.166 n=6
Emit Time1.66s (± 1.52%)1.65s (± 0.71%)~1.64s1.67sp=0.511 n=6
Total Time24.52s (± 0.27%)24.44s (± 0.14%)-0.08s (- 0.34%)24.40s24.49sp=0.030 n=6
ts-pre-modules - node (v18.15.0, x64)
Errors6868~~~p=1.000 n=6
Symbols225,018225,018~~~p=1.000 n=6
Types94,24994,249~~~p=1.000 n=6
Memory used370,192k (± 0.02%)370,274k (± 0.02%)~370,194k370,433kp=0.128 n=6
Parse Time2.76s (± 1.14%)2.77s (± 0.50%)~2.76s2.79sp=0.517 n=6
Bind Time1.58s (± 1.41%)1.57s (± 0.57%)~1.56s1.58sp=0.676 n=6
Check Time15.74s (± 0.35%)15.80s (± 0.38%)~15.73s15.90sp=0.126 n=6
Emit Time0.00s0.00s~~~p=1.000 n=6
Total Time20.08s (± 0.36%)20.14s (± 0.27%)~20.09s20.24sp=0.148 n=6
vscode - node (v18.15.0, x64)
Errors11~~~p=1.000 n=6
Symbols3,052,3183,052,376+58 (+ 0.00%)~~p=0.001 n=6
Types1,055,6221,055,643+21 (+ 0.00%)~~p=0.001 n=6
Memory used3,161,885k (± 0.00%)3,161,935k (± 0.00%)~3,161,880k3,161,998kp=0.093 n=6
Parse Time13.88s (± 0.23%)13.91s (± 0.37%)~13.85s13.97sp=0.332 n=6
Bind Time4.36s (± 0.24%)4.36s (± 0.19%)~4.35s4.37sp=0.923 n=6
Check Time81.54s (± 0.20%)81.79s (± 0.42%)~81.19s82.14sp=0.173 n=6
Emit Time22.20s (± 0.47%)22.16s (± 0.39%)~22.00s22.23sp=0.810 n=6
Total Time121.98s (± 0.21%)122.22s (± 0.25%)~121.68s122.62sp=0.173 n=6
webpack - node (v18.15.0, x64)
Errors00~~~p=1.000 n=6
Symbols277,156277,089-67 (- 0.02%)~~p=0.001 n=6
Types112,946113,102+156 (+ 0.14%)~~p=0.001 n=6
Memory used426,974k (± 0.02%)427,050k (± 0.03%)~426,860k427,189kp=0.230 n=6
Parse Time4.91s (± 0.47%)4.89s (± 0.27%)~4.87s4.91sp=0.164 n=6
Bind Time2.15s (± 0.88%)2.14s (± 1.54%)~2.10s2.19sp=0.625 n=6
Check Time21.81s (± 0.25%)21.78s (± 0.47%)~21.66s21.91sp=1.000 n=6
Emit Time0.00s0.00s~~~p=1.000 n=6
Total Time28.87s (± 0.18%)28.81s (± 0.42%)~28.66s28.94sp=0.575 n=6
xstate-main - node (v18.15.0, x64)
Errors00~~~p=1.000 n=6
Symbols539,317539,355+38 (+ 0.01%)~~p=0.001 n=6
Types178,997179,004+7 (+ 0.00%)~~p=0.001 n=6
Memory used484,083k (± 0.03%)484,223k (± 0.02%)~484,144k484,376kp=0.093 n=6
Parse Time4.26s (± 0.29%)4.26s (± 0.61%)~4.22s4.28sp=0.869 n=6
Bind Time1.55s (± 0.97%)1.55s (± 0.63%)~1.53s1.56sp=0.864 n=6
Check Time22.61s (± 0.43%)22.85s (± 0.40%)+0.24s (+ 1.06%)22.76s22.98sp=0.005 n=6
Emit Time0.00s0.00s~~~p=1.000 n=6
Total Time28.43s (± 0.37%)28.66s (± 0.34%)+0.23s (+ 0.80%)28.56s28.82sp=0.012 n=6
System info unknown
Hosts
  • node (v18.15.0, x64)
Scenarios
  • Compiler-Unions - node (v18.15.0, x64)
  • angular-1 - node (v18.15.0, x64)
  • mui-docs - node (v18.15.0, x64)
  • self-build-src - node (v18.15.0, x64)
  • self-build-src-public-api - node (v18.15.0, x64)
  • self-compiler - node (v18.15.0, x64)
  • ts-pre-modules - node (v18.15.0, x64)
  • vscode - node (v18.15.0, x64)
  • webpack - node (v18.15.0, x64)
  • xstate-main - node (v18.15.0, x64)
BenchmarkNameIterations
Currentpr6
Baselinebaseline6

Developer Information:

Download Benchmarks

@weswigham
Copy link
MemberAuthor

There we go, no more new errors in angular - now just to see if top400 comes back cleaner, too. (I think it should, the most recent fix is squarely aimed at those errors, since they're the same cause as the ones in angular - forgetting to include the explicit prop types in the implied index signature type)

@typescript-bot
Copy link
Collaborator

@weswigham Here are the results of running the top 400 repos with tsc comparingmain andrefs/pull/59860/merge:

Everything looks good!

@weswighamweswigham marked this pull request as ready for reviewSeptember 16, 2024 17:01
Comment on lines +16313 to +16315
if (hasComputedStringProperty && !findIndexInfo(indexInfos, stringType)) indexInfos.push(getObjectLiteralIndexInfo(readonlyComputedStringProperty, 0, allPropertySymbols, stringType));
if (hasComputedNumberProperty && !findIndexInfo(indexInfos, numberType)) indexInfos.push(getObjectLiteralIndexInfo(readonlyComputedNumberProperty, 0, allPropertySymbols, numberType));
if (hasComputedSymbolProperty && !findIndexInfo(indexInfos, esSymbolType)) indexInfos.push(getObjectLiteralIndexInfo(readonlyComputedSymbolProperty, 0, allPropertySymbols, esSymbolType));
Copy link
Member

Choose a reason for hiding this comment

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

Are all of thefindIndexInfo calls likely to cost a bunch or isindexInfos just always short?

Copy link
MemberAuthor

@weswighamweswighamSep 18, 2024•
edited
Loading

Choose a reason for hiding this comment

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

In practice, it's always short (there's usually juststring ornumber indexes, if any) - in theory, a user could explicitly define hundreds of pattern literal index infos and it'd be bad, however, and you'd prefer you did the up-front work to make a map keyed on key type (if you had multiple kinds of computed names). But that's not code anyone's realistically writing.

@weswighamweswigham added the Breaking ChangeWould introduce errors in existing code labelSep 23, 2024
@weswighamweswigham merged commitfa0080f intomicrosoft:mainSep 23, 2024
32 checks passed
@pfumagalli
Copy link

Hmm... I hit a bit of a snag here. I have a class like this:

importutilfrom'node:util'constinspect=util.inspect.customexportclassMyMapextendsMap<MyObjectA,MyObjectB>{[inspect](depth:number,options:util.InspectOptions):string{// my fancy code for making this look pretty...}// ... and here goes all the extra stuff I want}

Now this compiles out as a.d.ts as:

exportdeclareclassMyMapextendsMap<MyObjectA,MyObjectB>{[x:symbol]:((depth:number,options:util.InspectOptions)=>string)|(()=>MapIterator<[string,string]>);// ... all the other niceties}

... so far so good (maybe???) but when I try to import my map in some other project I get an error:

Property '[Symbol.toStringTag]' of type 'string' is not assignable to 'symbol' index type '((depth: number, options: InspectOptions) => string) | (() => MapIterator<[string, string]>)'. [TS2411]    [x: symbol]: ((depth: number, options: util.InspectOptions) => string) | (() => MapIterator<[string, string]>);

Why? Because `` declaresMap as follows, which is obviously incompatible:

interfaceMap<K,V>{readonly[Symbol.toStringTag]:string;}

Maybe the typescript compiler should merge all symbol declarations and write something like:

exportdeclareclassMyMapextendsMap<MyObjectA,MyObjectB>{[x:symbol]:string|((depth:number,options:util.InspectOptions)=>string)|(()=>MapIterator<[string,string]>);// ... all the other niceties}

I don't know what'd be theright solution in these cases.

For my code, I can simply remove that assignmentconst inspect = util.inspect.custom and use it directly in my code:

importutilfrom'node:util'exportclassMyMapextendsMap<MyObjectA,MyObjectB>{[util.inspect.custom](depth:number,options:util.InspectOptions):string{// my fancy code for making this look pretty...}// ... and here goes all the extra stuff I want}

produces a more correct.d.ts as follows (which makes dependant libraries work):

importutilfrom'node:util';exportdeclareclassMyMapextendsMap<MyObjectA,MyObjectB>{[util.inspect.custom]:((depth:number,options:util.InspectOptions)=>string)|(()=>MapIterator<[string,string]>);// ... all the other niceties}

But I noticed that anytime I import aunique symbol (such as in the case ofutils.inspect.custom) assign it locally, and use it as a key in a class, theuniqueness of the symbol is somehow lost and the whole thing collapses...

@farfromrefug
Copy link

@weswigham i have an issue with that change.
I have a class like this:

classProperty{readonlygetDefault:symbol;constructor(propertyName:string){constgetDefault=Symbol(propertyName+':getDefault');this.getDefault=getDefault;}}

And in another class i do this:

constmyProp=newProperty()classA{[myProp.getDefault]():boolean{returntrue;}}

Before TS 5.7 it was working correctly. Now because of this PR it reports an error:

Property '[myProperty.getDefault]' of type '() => boolean' is not assignable to 'symbol' index type '() => string'.ts(2411)

And i really dont see how to fix it. Can you help?

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@jakebaileyjakebaileyjakebailey approved these changes

@andrewbranchandrewbranchAwaiting requested review from andrewbranch

@rbucktonrbucktonAwaiting requested review from rbuckton

Assignees

@weswighamweswigham

Labels
Author: TeamBreaking ChangeWould introduce errors in existing codeFor Uncommitted BugPR for untriaged, rejected, closed or missing bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

5 participants
@weswigham@typescript-bot@jakebailey@pfumagalli@farfromrefug
[8]ページ先頭
©2009-2025 Movatter.jp