preserve declarations if @internal is mentioned in unrelated comment#57960
preserve declarations if @internal is mentioned in unrelated comment#57960Zzzen wants to merge 10 commits intomicrosoft:mainfrom
Conversation
|
Looks like you're introducing a change to the public API surface area. If this includes breaking changes, please document them on our wiki's API Breaking Changes page. Also, please make sure @DanielRosenwasser and @RyanCavanaugh are aware of the changes, just as a heads up. |
Co-authored-by: Jake Bailey <[email protected]>
|
Merged main for you. As far as I'm concerned, this PR is correct; I tested But I know we're pretty iffy on whether or not I'm personally still for it, regardless, but I'll bring this one up in a design meeting just to double check. |
jakebailey
left a comment
There was a problem hiding this comment.
I have no idea how it isn't breaking the tests, but this PR will need to modify scanner.ts to update jsDocSeeOrLink to also treat @internal as a JSDoc comment that needs to be parsed in TS files and an update to the docs for JSDocParsingMode to say that @internal is also parsed. Along with testing in jsDocParsing.ts.
Probably need to make ParseForTypeErrors the default in the compiler tests... Really thought I did that already.
jakebailey
left a comment
There was a problem hiding this comment.
Oops, meant to mark as needing changes.
|
Close/reopening to rerun CI and show the failure. |
|
Took the liberty to apply the described change. |
|
Just checking: @typescript-bot test it |
|
Hey @jakebailey, the results of running the DT tests are ready. There were interesting changes: Branch only errors:Package: react Package: porty |
jakebailey
left a comment
There was a problem hiding this comment.
Those new DT errors are interesting; seems like a bug in the PR somewhere.
|
@jakebailey Here are the results of running the user tests with tsc comparing Something interesting changed - please have a look. Details
|
|
@jakebailey Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@jakebailey Here are the results of running the top 400 repos with tsc comparing Everything looks good! |
|
The DT errors should have been fixed. |
|
@typescript-bot test it |
|
@typescript-bot test it |
|
Hey @jakebailey, the results of running the DT tests are ready. Everything looks the same! |
|
@jakebailey Here are the results of running the user tests with tsc comparing Everything looks good! |
|
@jakebailey Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Perf is unfortunately still greatly affected and needs to be looked into. |
|
@jakebailey Here are the results of running the top 400 repos with tsc comparing Everything looks good! |
|
Perf problem is likely caused by the regex change.
|
|
Funnily I clicked that link and reran it, and it told me that the new one was 6% faster; I think this particular result is just noisy. Given the benchmark is consistent with so few samples, a profile may provide more insight (which I plan to test). |
|
LOL. After I executed it many times, I also found that the output result is somewhat random. |
|
Do the random perf results mean this is safe to merge? I'd guess not, unless it's a problem with the perf infrastructure. |
|
No, there's nothing wrong with the perf infra, there really is a slowdown here that needs to be profiled. |
|
@typescript-bot perf test this faster I can't seem to get the slowdown locally... |
|
@jakebailey Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
With 6.0 out as the final release vehicle for this codebase, we're closing all PRs that don't fit the merge criteria for post-6.0 patches. If you think this was a mistake and this PR fits the post-6.0 patch criteria, please post to the 6.0 iteration issue with details (specifically, which PR and which patch criteria it satisfies). Next steps for PRs:
|
Fixes #57352