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

Revert "Revert "Fixed an issue with contextual type for intersection … #50311

Closed
wants to merge 1 commit into from

Conversation

amcasey
Copy link
Member

@amcasey amcasey commented Aug 15, 2022

…properties (#48668)" (#50279)"

This reverts commit adf26ff.

I'd like to see what the on-demand user tests report for this

@amcasey amcasey marked this pull request as draft August 15, 2022 23:45
@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Aug 15, 2022
@amcasey
Copy link
Member Author

amcasey commented Aug 15, 2022

@typescript-bot user test this inline

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 15, 2022

Heya @amcasey, I'm starting to run the diff-based user code test suite on this PR at b6bb411. Hold tight - I'll update this comment with the log link once the build has been queued.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

@amcasey Here are the results of running the user test suite comparing main and refs/pull/50311/merge

Everything looks good!

Run logs

@Andarist
Copy link
Contributor

Is there any other extended test suite that we could run against this? Those [NewErrors] issues created by the @typescript-bot have to run against more things, right? 🤔

@amcasey
Copy link
Member Author

amcasey commented Aug 16, 2022

@Andarist Sorry for the noise - I'm not actually investigating your change specifically - I'm just messing with the infrastructure and wanted to do some validation.

But, yes, it's possible to run individual PRs against the NewErrors infrastructure. I'll give that a shot once I get some more basic things working.

@Andarist
Copy link
Contributor

@Andarist Sorry for the noise - I'm not actually investigating your change specifically

oh… that’s a bummer… for me 😜

But, yes, it's possible to run individual PRs against the NewErrors infrastructure. I'll give that a shot once I get some more basic things working.

that’s pretty cool! If u ever get to it - please ping me, i’d be interested in those results

@typescript-bot
Copy link
Collaborator

@amcasey Here are the results of running the top-repos suite comparing main and refs/pull/50311/merge:

Something interesting changed - please have a look.

Details

reduxjs/redux

3 of 5 projects failed to build with the old tsc and were ignored

examples/counter-ts/tsconfig.json

@microsoft microsoft deleted a comment from typescript-bot Aug 17, 2022
@microsoft microsoft deleted a comment from typescript-bot Aug 17, 2022
@microsoft microsoft deleted a comment from typescript-bot Aug 17, 2022
@microsoft microsoft deleted a comment from typescript-bot Aug 17, 2022
@microsoft microsoft deleted a comment from typescript-bot Aug 17, 2022
@microsoft microsoft deleted a comment from typescript-bot Aug 17, 2022
@microsoft microsoft deleted a comment from typescript-bot Aug 17, 2022
@microsoft microsoft deleted a comment from typescript-bot Aug 17, 2022
@microsoft microsoft deleted a comment from typescript-bot Aug 17, 2022
@microsoft microsoft deleted a comment from typescript-bot Aug 17, 2022
@microsoft microsoft deleted a comment from typescript-bot Aug 17, 2022
@microsoft microsoft deleted a comment from typescript-bot Aug 17, 2022
@microsoft microsoft deleted a comment from typescript-bot Aug 17, 2022
@amcasey
Copy link
Member Author

amcasey commented Aug 17, 2022

@Andarist the testing infrastructure is still in flux, but I think the bot comment I didn't delete shows the problem that necessitated the revert.

@Andarist
Copy link
Contributor

@amcasey yes, the Redux types were the reason behind the revert - so this is expected. It's good to know though that "only" Redux was affected by this change.

@amcasey
Copy link
Member Author

amcasey commented Aug 17, 2022

@Andarist Nope, no new information, but you sounded curious. 😄

The PR-specific tests only cover the top 100 GH repos (by star count) but the next 100 show many more breaks (though most/all related to their consumption of redux, as far as I know). Mostly, I'm pleased to know that we have a system in place now that could have caught the issue.

@amcasey amcasey closed this Aug 17, 2022
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.

None yet

3 participants