Movatterモバイル変換

typescript-bot added Author: Team For Uncommitted BugPR for untriaged, rejected, closed or missing bug labels

Copy link

MemberAuthor

jakebailey commentedSep 21, 2023

@typescript-bot test top100
@typescript-bot user test this
@typescript-bot run dt
@typescript-bot perf test this faster
@typescript-bot perf test this bun
@typescript-bot pack this

Copy link

Collaborator

typescript-bot commentedSep 21, 2023•
edited
Loading

Heya@jakebailey, I've started to run the parallelized Definitely Typed test suite on this PR at83275d2. You can monitor the buildhere.

Copy link

Collaborator

typescript-bot commentedSep 21, 2023•
edited
Loading

Heya@jakebailey, I've started to run the diff-based user code test suite on this PR at83275d2. You can monitor the buildhere.

Copy link

Collaborator

typescript-bot commentedSep 21, 2023•
edited
Loading

Heya@jakebailey, I've started to run the tsc-only perf test suite on this PR at83275d2. You can monitor the buildhere.

Copy link

Collaborator

typescript-bot commentedSep 21, 2023•
edited
Loading

Heya@jakebailey, I've started to run the bun perf test suite on this PR at83275d2. You can monitor the buildhere.

Copy link

Collaborator

typescript-bot commentedSep 21, 2023•
edited
Loading

Heya@jakebailey, I've started to run the tarball bundle task on this PR at83275d2. You can monitor the buildhere.

Copy link

Collaborator

typescript-bot commentedSep 21, 2023•
edited
Loading

Heya@jakebailey, I've started to run the diff-based top-repos suite on this PR at83275d2. You can monitor the buildhere.

Copy link

Collaborator

typescript-bot commentedSep 21, 2023•
edited
Loading

Hey@jakebailey, 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/157853/artifacts?artifactName=tgz&fileId=C5FE361F795082FBDD84BBEDFDAF0A2FD34FDA502A39BDFA74E19E895FF4773102&fileName=/typescript-5.3.0-insiders.20230921.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.3.0-pr-55820-7".;

Copy link

Collaborator

typescript-bot commentedSep 21, 2023

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

Here they are:

Compiler

Comparison Report - baseline..pr

_Metric	_baseline	_pr	_Delta	_Best	_Worst	_p-value
_{Angular - node (v18.15.0, x64)}
_{Memory used}	_{294,968k (± 0.00%)}	_{294,935k (± 0.00%)}	_{-33k (- 0.01%)}	_294,926k	_294,947k	_{p=0.005 n=6}
_{Parse Time}	_{2.62s (± 0.52%)}	_{2.63s (± 0.32%)}	_~	_2.62s	_2.64s	_{p=0.666 n=6}
_{Bind Time}	_{0.84s (± 1.23%)}	_{0.84s (± 1.30%)}	_~	_0.83s	_0.85s	_{p=0.640 n=6}
_{Check Time}	_{8.01s (± 0.21%)}	_{8.00s (± 0.37%)}	_~	_7.97s	_8.04s	_{p=0.566 n=6}
_{Emit Time}	_{7.02s (± 0.18%)}	_{7.04s (± 0.23%)}	_~	_7.02s	_7.06s	_{p=0.070 n=6}
_{Total Time}	_{18.49s (± 0.13%)}	_{18.50s (± 0.09%)}	_~	_18.47s	_18.52s	_{p=0.871 n=6}
_{Compiler-Unions - node (v18.15.0, x64)}
_{Memory used}	_{191,606k (± 1.19%)}	_{192,551k (± 1.56%)}	_~	_190,602k	_196,550k	_{p=1.000 n=6}
_{Parse Time}	_{1.35s (± 0.47%)}	_{1.35s (± 0.47%)}	_~	_1.34s	_1.36s	_{p=1.000 n=6}
_{Bind Time}	_{0.73s (± 0.00%)}	_{0.73s (± 0.00%)}	_~	_0.73s	_0.73s	_{p=1.000 n=6}
_{Check Time}	_{9.11s (± 0.70%)}	_{9.07s (± 0.29%)}	_~	_9.05s	_9.12s	_{p=0.326 n=6}
_{Emit Time}	_{2.60s (± 0.60%)}	_{2.60s (± 0.68%)}	_~	_2.58s	_2.63s	_{p=0.685 n=6}
_{Total Time}	_{13.79s (± 0.44%)}	_{13.75s (± 0.26%)}	_~	_13.70s	_13.81s	_{p=0.244 n=6}
_{Monaco - node (v18.15.0, x64)}
_{Memory used}	_{347,206k (± 0.01%)}	_{347,204k (± 0.00%)}	_~	_347,194k	_347,213k	_{p=0.936 n=6}
_{Parse Time}	_{2.46s (± 0.55%)}	_{2.46s (± 0.55%)}	_~	_2.44s	_2.48s	_{p=0.934 n=6}
_{Bind Time}	_{0.94s (± 0.00%)}	_{0.94s (± 0.86%)}	_~	_0.94s	_0.96s	_{p=0.405 n=6}
_{Check Time}	_{6.84s (± 0.39%)}	_{6.86s (± 0.44%)}	_~	_6.82s	_6.90s	_{p=0.195 n=6}
_{Emit Time}	_{4.04s (± 0.71%)}	_{4.04s (± 0.50%)}	_~	_4.01s	_4.06s	_{p=0.871 n=6}
_{Total Time}	_{14.28s (± 0.32%)}	_{14.30s (± 0.21%)}	_~	_14.26s	_14.34s	_{p=0.261 n=6}
_{TFS - node (v18.15.0, x64)}
_{Memory used}	_{302,465k (± 0.00%)}	_{302,474k (± 0.01%)}	_~	_302,454k	_302,494k	_{p=0.336 n=6}
_{Parse Time}	_{2.01s (± 0.94%)}	_{2.00s (± 0.93%)}	_~	_1.99s	_2.04s	_{p=0.249 n=6}
_{Bind Time}	_{1.00s (± 0.63%)}	_{1.00s (± 0.75%)}	_~	_0.99s	_1.01s	_{p=0.718 n=6}
_{Check Time}	_{6.23s (± 0.33%)}	_{6.25s (± 0.22%)}	_~	_6.23s	_6.26s	_{p=0.363 n=6}
_{Emit Time}	_{3.52s (± 1.00%)}	_{3.52s (± 0.69%)}	_~	_3.49s	_3.55s	_{p=0.936 n=6}
_{Total Time}	_{12.76s (± 0.25%)}	_{12.76s (± 0.29%)}	_~	_12.71s	_12.81s	_{p=0.872 n=6}
_{material-ui - node (v18.15.0, x64)}
_{Memory used}	_{470,438k (± 0.00%)}	_{470,450k (± 0.00%)}	_~	_470,414k	_470,476k	_{p=0.173 n=6}
_{Parse Time}	_{2.58s (± 0.41%)}	_{2.58s (± 0.76%)}	_~	_2.55s	_2.60s	_{p=1.000 n=6}
_{Bind Time}	_{0.99s (± 1.22%)}	_{0.99s (± 0.76%)}	_~	_0.98s	_1.00s	_{p=0.503 n=6}
_{Check Time}	_{16.59s (± 0.41%)}	_{16.57s (± 0.23%)}	_~	_16.52s	_16.63s	_{p=0.630 n=6}
_{Emit Time}	_{0.00s (± 0.00%)}	_{0.00s (± 0.00%)}	_~	_0.00s	_0.00s	_{p=1.000 n=6}
_{Total Time}	_{20.16s (± 0.30%)}	_{20.14s (± 0.16%)}	_~	_20.11s	_20.20s	_{p=0.334 n=6}
_{xstate - node (v18.15.0, x64)}
_{Memory used}	_{512,489k (± 0.01%)}	_{512,494k (± 0.02%)}	_~	_512,373k	_512,691k	_{p=0.936 n=6}
_{Parse Time}	_{3.26s (± 0.37%)}	_{3.27s (± 0.46%)}	_~	_3.24s	_3.28s	_{p=0.218 n=6}
_{Bind Time}	_{1.55s (± 0.33%)}	_{1.55s (± 0.41%)}	_~	_1.54s	_1.56s	_{p=0.386 n=6}
_{Check Time}	_{2.81s (± 1.09%)}	_{2.78s (± 0.23%)}	_~	_2.77s	_2.79s	_{p=0.059 n=6}
_{Emit Time}	_{0.08s (± 4.99%)}	_{0.08s (± 4.99%)}	_~	_0.08s	_0.09s	_{p=1.000 n=6}
_{Total Time}	_{7.69s (± 0.45%)}	_{7.68s (± 0.27%)}	_~	_7.66s	_7.71s	_{p=0.628 n=6}

System info unknown

Hosts

node (v18.15.0, x64)

Scenarios

Angular - node (v18.15.0, x64)
Compiler-Unions - node (v18.15.0, x64)
Monaco - node (v18.15.0, x64)
TFS - node (v18.15.0, x64)
material-ui - node (v18.15.0, x64)
xstate - node (v18.15.0, x64)

_Benchmark	_Name	_Iterations
_Current	_pr	₆
_Baseline	_baseline	₆

Developer Information:

Download Benchmarks

Copy link

Collaborator

typescript-bot commentedSep 21, 2023

@jakebailey Here are the results of running the user test suite comparingmain andrefs/pull/55820/merge:

There were infrastructure failures potentially unrelated to your change:

1 instance of "Unknown failure"
2 instances of "Package install failed"

Otherwise...

Everything looks good!

jakebailey commented

src/compiler/types.ts

		spreadIndices?:{first:number\|undefined,last:number\|undefined};// Indices of first and last spread elements in array literal
		parameterInitializerContainsUndefined?:boolean;// True if this is a parameter declaration whose type annotation contains "undefined".
		fakeScopeForSignatureDeclaration?:boolean;//True if this is a fake scope injected into an enclosing declaration chain.
		fakeScopeForSignatureDeclaration?:"params"\|"typeParams";//If present, this is a fake scope injected into an enclosing declaration chain.

Copy link

MemberAuthor

jakebaileySep 21, 2023

Choose a reason for hiding this comment

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

Not sure if this should be an enum, I just used a string here as the least annoying way to get two truthy values.

Copy link

Collaborator

typescript-bot commentedSep 21, 2023

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

Here they are:

Compiler

Comparison Report - baseline..pr

_Metric	_baseline	_pr	_Delta	_Best	_Worst	_p-value
_{Angular - bun (v1.0.2, x64)}
_{Memory used}	_{317,145k (± 0.26%)}	_{316,439k (± 0.24%)}	_~	_314,201k	_318,067k	_{p=0.126 n=12}
_{Parse Time}	_{2.29s (± 0.48%)}	_{2.29s (± 0.47%)}	_~	_2.27s	_2.33s	_{p=1.000 n=12}
_{Bind Time}	_{0.90s (± 1.69%)}	_{0.89s (± 1.01%)}	_~	_0.87s	_0.92s	_{p=0.745 n=12}
_{Check Time}	_{7.98s (± 0.45%)}	_{7.99s (± 0.25%)}	_~	_7.94s	_8.04s	_{p=0.259 n=12}
_{Emit Time}	_{6.43s (± 0.45%)}	_{6.41s (± 0.30%)}	_~	_6.35s	_6.47s	_{p=0.307 n=12}
_{Total Time}	_{17.59s (± 0.30%)}	_{17.58s (± 0.16%)}	_~	_17.53s	_17.67s	_{p=0.885 n=12}
_{Compiler-Unions - bun (v1.0.2, x64)}
_{Memory used}	_{243,470k (± 1.02%)}	_{245,379k (± 0.59%)}	_~	_239,833k	_247,542k	_{p=0.371 n=12}
_{Parse Time}	_{1.13s (± 1.66%)}	_{1.13s (± 1.41%)}	_~	_1.11s	_1.18s	_{p=1.000 n=12}
_{Bind Time}	_{0.84s (± 0.67%)}	_{0.84s (± 2.16%)}	_~	_0.81s	_0.90s	_{p=0.498 n=12}
_{Check Time}	_{8.82s (± 0.53%)}	_{8.78s (± 0.18%)}	_~	_8.74s	_8.82s	_{p=0.352 n=12}
_{Emit Time}	_{2.78s (± 0.61%)}	_{2.79s (± 0.35%)}	_~	_2.76s	_2.81s	_{p=0.320 n=12}
_{Total Time}	_{13.57s (± 0.37%)}	_{13.55s (± 0.20%)}	_~	_13.48s	_13.62s	_{p=0.369 n=12}
_{Monaco - bun (v1.0.2, x64)}
_{Memory used}	_{377,339k (± 0.21%)}	_{377,447k (± 0.18%)}	_~	_375,870k	_379,065k	_{p=0.795 n=12}
_{Parse Time}	_{1.97s (± 0.64%)}	_{1.97s (± 0.67%)}	_~	_1.95s	_2.00s	_{p=0.930 n=12}
_{Bind Time}	_{0.96s (± 1.48%)}	_{0.97s (± 1.81%)}	_~	_0.93s	_1.02s	_{p=0.268 n=12}
_{Check Time}	_{7.38s (± 0.30%)}	_{7.37s (± 0.27%)}	_~	_7.31s	_7.41s	_{p=0.543 n=12}
_{Emit Time}	_{3.78s (± 0.47%)}	_{3.77s (± 0.70%)}	_~	_3.69s	_3.86s	_{p=0.211 n=12}
_{Total Time}	_{14.10s (± 0.18%)}	_{14.08s (± 0.23%)}	_~	_14.01s	_14.19s	_{p=0.434 n=12}
_{TFS - bun (v1.0.2, x64)}
_{Memory used}	_{317,938k (± 0.20%)}	_{317,329k (± 0.17%)}	_~	_315,501k	_318,466k	_{p=0.157 n=12}
_{Parse Time}	_{1.74s (± 0.33%)}	_{1.75s (± 0.73%)}	_~	_1.73s	_1.80s	_{p=0.567 n=12}
_{Bind Time}	_{0.99s (± 1.41%)}	_{0.98s (± 1.88%)}	_~	_0.92s	_1.01s	_{p=0.930 n=12}
_{Check Time}	_{6.68s (± 0.42%)}	_{6.68s (± 0.36%)}	_~	_6.61s	_6.73s	_{p=0.908 n=12}
_{Emit Time}	_{3.47s (± 0.71%)}	_{3.47s (± 0.70%)}	_~	_3.42s	_3.55s	_{p=0.908 n=12}
_{Total Time}	_{12.88s (± 0.36%)}	_{12.89s (± 0.31%)}	_~	_12.79s	_12.98s	_{p=0.622 n=12}
_{material-ui - bun (v1.0.2, x64)}
_{Memory used}	_{494,317k (± 1.91%)}	_{484,418k (± 1.60%)}	_~	_475,405k	_510,266k	_{p=0.078 n=12}
_{Parse Time}	_{2.23s (± 0.33%)}	_{2.22s (± 0.28%)}	_~	_2.21s	_2.24s	_{p=0.952 n=12}
_{Bind Time}	_{0.74s (± 3.03%)}	_{0.76s (± 1.96%)}	_~	_0.71s	_0.78s	_{p=0.201 n=12}
_{Check Time}	_{15.80s (± 0.40%)}	_{15.87s (± 0.47%)}	_~	_15.70s	_16.05s	_{p=0.119 n=12}
_{Emit Time}	_{0.00s (± 0.00%)}	_{0.00s (± 0.00%)}	_~	_0.00s	_0.00s	_{p=1.000 n=12}
_{Total Time}	_{18.76s (± 0.29%)}	_{18.86s (± 0.41%)}	_{+0.10s (+ 0.52%)}	_18.68s	_19.01s	_{p=0.049 n=12}
_{xstate - bun (v1.0.2, x64)}
_{Memory used}	_{410,202k (± 5.77%)}	_{403,990k (± 5.55%)}	_~	_381,218k	_463,907k	_{p=0.707 n=12}
_{Parse Time}	_{3.22s (± 0.39%)}	_{3.23s (± 0.53%)}	_~	_3.20s	_3.28s	_{p=0.205 n=12}
_{Bind Time}	_{1.26s (± 1.03%)}	_{1.26s (± 0.63%)}	_~	_1.24s	_1.28s	_{p=0.814 n=12}
_{Check Time}	_{3.59s (± 0.28%)}	_{3.59s (± 0.35%)}	_~	_3.56s	_3.62s	_{p=0.347 n=12}
_{Emit Time}	_{0.21s (± 1.57%)}	_{0.21s (± 2.93%)}	_~	_0.20s	_0.23s	_{p=0.974 n=12}
_{Total Time}	_{8.28s (± 0.29%)}	_{8.29s (± 0.20%)}	_~	_8.25s	_8.35s	_{p=0.224 n=12}

System info unknown

Hosts

bun (v1.0.2, x64)

Scenarios

Angular - bun (v1.0.2, x64)
Compiler-Unions - bun (v1.0.2, x64)
Monaco - bun (v1.0.2, x64)
TFS - bun (v1.0.2, x64)
material-ui - bun (v1.0.2, x64)
xstate - bun (v1.0.2, x64)

_Benchmark	_Name	_Iterations
_Current	_pr	₁₂
_Baseline	_baseline	₁₂

Startup

Comparison Report - baseline..pr

_Metric	_baseline	_pr	_Delta	_Best	_Worst	_p-value
_{tsc-startup - bun (v1.0.2, x64)}
_{Execution time}	_{425.46ms (± 0.08%)}	_{425.43ms (± 0.10%)}	_~	_424.08ms	_433.95ms	_{p=0.051 n=600}
_{tsserverlibrary-startup - bun (v1.0.2, x64)}
_{Execution time}	_{660.84ms (± 0.12%)}	_{662.09ms (± 0.13%)}	_{+1.25ms (+ 0.19%)}	_660.21ms	_685.95ms	_{p=0.000 n=600}
_{typescript-startup - bun (v1.0.2, x64)}
_{Execution time}	_{659.87ms (± 0.12%)}	_{661.14ms (± 0.13%)}	_{+1.27ms (+ 0.19%)}	_659.39ms	_676.29ms	_{p=0.000 n=600}

System info unknown

Hosts

bun (v1.0.2, x64)

Scenarios

tsc-startup - bun (v1.0.2, x64)
tsserverlibrary-startup - bun (v1.0.2, x64)
typescript-startup - bun (v1.0.2, x64)

_Benchmark	_Name	_Iterations
_Current	_pr	₆
_Baseline	_baseline	₆

Developer Information:

Download Benchmarks

Copy link

Collaborator

typescript-bot commentedSep 21, 2023

@jakebailey Here are the results of running the top-repos suite comparingmain andrefs/pull/55820/merge:

Everything looks good!

Copy link

Collaborator

typescript-bot commentedSep 21, 2023

Hey@jakebailey, the results of running the DT tests are ready.
Everything looks the same!
You can check the log here.

jakebailey mentioned this pull request

Enable GenerateNamesForShadowedTypeParams for types baselines#55821

Merged

Copy link

fatcerberus commentedSep 22, 2023•
edited
Loading

Preserve shadowed type parameter names

Looking at#55653, technically it's theother type that's being shadowed by the type parameter, and that one alreadyis preserved (the type parameter is renamed). If it were the other way around then the current behavior might actually be correct 😉

Copy link

fatcerberus commentedSep 22, 2023•
edited
Loading

Here's a test, in case this isn't already covered:

classA{}consta=newA();constb=<A,>(x:A)=>a

The type parameter still needs to be renamed here.

Also here's a fun bit of chicanery:

Looks like an identity function, but isn't!

Copy link

MemberAuthor

jakebailey commentedSep 22, 2023•
edited
Loading

This PR makes that work, check the declaration tab here:https://www.staging-typescript.org/play?ts=5.3.0-pr-55820-7#code/MYGwhgzhAECC0G9oF8BQwD2A7CAXaY0AvNFgKYDucAFAJQDc62e0ARsdADywA0AfNQAeALji1ifAkA

That's actually the test case I added first.

Unfortunately hover doesn't set the flag from:#55821 I don't think. I've been wondering if we should just drop this flag, so we are consistent...

Copy link

fatcerberus commentedSep 22, 2023

declare const b: <A>(x: A) => globalThis.A;

This is a legal declaration? I didn't knowglobalThis existed in type-space.

Also if the file is a module the output is... questionable. I'd rather just have the type parameter be renamed here.

exportdeclareclassA{}exportdeclareconsta:A;exportdeclareconstb:<A>(x:A)=>import("./input").A;

Copy link

MemberAuthor

jakebailey commentedSep 22, 2023

Write:

classA{}consta=newA();functionb<A,>(x:A){returna;}

And you'll see that we already emitglobalThis.A; the only reason we don't now is because declaration emit andsignature to node emit are different.

Copy link

fatcerberus commentedSep 22, 2023

Hmm, you're right, thanks. That self-import in the case of a module still bugs mea lot, though.

jakebailey changed the title~~Preserve shadowed type parameter names in more cases~~Preserve original type parameter names more often when shadowed

Sep 22, 2023

jakebailey marked this pull request as draft

September 24, 2023 22:27

jakebailey mentioned this pull request

Oct 9, 2023

Reuse more input nodes when they contain local type parameter references#56046

Closed

jakebailey commented

Oct 9, 2023

tests/baselines/reference/declarationEmitTypeParameterNameShadowedInternally.js



		//// [declarationEmitTypeParameterNameShadowedInternally.d.ts]
		exportdeclareconstfoo:<T>(x:T)=><T_1>(y:T_1)=>readonly[T,T_1];

Copy link

MemberAuthor

jakebaileyOct 9, 2023

Choose a reason for hiding this comment

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

Actually I thought this test had failed, but actually, I don't think it did fail; this matches main, I think...

jakebailey commented

Oct 9, 2023

tests/baselines/reference/declarationEmitTypeParameterNameShadowedInternally.types Outdated

		>y : T

		return inner;
		>inner : <T>(y: T) => readonly [T, T]

Copy link

MemberAuthor

jakebaileyOct 9, 2023•
edited
Loading

Choose a reason for hiding this comment

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

Here you can see that the tooltip is very wrong because we don't enable renaming in tooltips.

Copy link

MemberAuthor

jakebaileyOct 9, 2023

Choose a reason for hiding this comment

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

Nothing new, though:

jakebailey marked this pull request as ready for review

October 9, 2023 21:03

jakebailey force-pushed thefix-55653 branch fromde29cc3 to5fb8ebbCompare

October 9, 2023 21:17

jakebailey force-pushed thefix-55653 branch froma9525a3 to889812bCompare

November 3, 2023 22:06

weswigham approved these changes

Nov 6, 2023