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.

Sign up for GitHub

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

Jump to bottom

Consider inferences between mapped type templates lower priority #44126

Merged
merged 1 commit into from May 29, 2021
Merged

Consider inferences between mapped type templates lower priority #44126

merged 1 commit into from May 29, 2021

Conversation

weswigham
Copy link
Member

Similarly to how inferences between mapped type constraints are lower priority. In general, inferences through a mapped type are usually just worse than other inferences, if available, due to the (potentially lossy) mapping operation the mapped type is doing.

Fixes #44092

@typescript-bot typescript-bot added Author: Team For Milestone Bug PRs that fix a bug with a specific milestone labels May 17, 2021
@weswigham weswigham mentioned this pull request May 17, 2021
Closed
@weswigham
Copy link
Member Author

@typescript-bot user test this
@typescript-bot test this
@typescript-bot run dt

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 17, 2021 •
edited

Heya @weswigham, I've started to run the extended test suite on this PR at 99110e9. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 17, 2021 •
edited

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 17, 2021 •
edited

Heya @weswigham, I've started to run the parallelized community code test suite on this PR at 99110e9. You can monitor the build here.

@typescript-bot typescript-bot mentioned this pull request May 17, 2021
Open
@typescript-bot
Copy link
Collaborator

The user suite test run you requested has finished and failed. I've opened a PR with the baseline diff from master.

sandersn
sandersn reviewed May 18, 2021
View reviewed changes
Copy link
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

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

The lower priority makes sense to me, but it seems like a risky change to make in the RC because it will likely result in another complex followup bug. I vote to hold this for 4.4 unless you know that the problem is widespread.

@DanielRosenwasser DanielRosenwasser mentioned this pull request May 18, 2021
Open
@DanielRosenwasser
Copy link
Member

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 18, 2021 •
edited

Heya @DanielRosenwasser, I've started to run the tarball bundle task on this PR at 99110e9. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 18, 2021 •
edited

Hey @DanielRosenwasser, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/103289/artifacts?artifactName=tgz&fileId=A55A16C587F60DF7E2A4B95F02939F25AB0C274D50F5790CB101D9A07CCF6AD902&fileName=/typescript-4.4.0-insiders.20210518.tgz"
    }
}

and then running npm install.

There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/pr-build@4.4.0-pr-44126-7".;

@DanielRosenwasser
Copy link
Member

@typescript-bot cherry-pick this to release-4.3

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 20, 2021 •
edited

Heya @DanielRosenwasser, I've started to run the task to cherry-pick this into release-4.3 on this PR at 99110e9. You can monitor the build here.

@typescript-bot typescript-bot mentioned this pull request May 20, 2021
Merged
@typescript-bot
Copy link
Collaborator

Hey @DanielRosenwasser, I've opened #44189 for you.

typescript-bot pushed a commit to typescript-bot/TypeScript that referenced this pull request May 20, 2021
@weswigham
Cherry-pick PR microsoft#44126 into release-4.3
90301b2 
Component commits:
99110e9 Consider inferences between mapped type templates lower priority
@sandersn sandersn added this to Not started in PR Backlog May 28, 2021
@sandersn
Copy link
Member

@weswigham @DanielRosenwasser Is this ready to go into master and 4.3 respectively?

@sandersn sandersn moved this from Not started to Needs merge in PR Backlog May 28, 2021
@weswigham
Copy link
Member Author

I mean, we should probably ship it in the nightly for a bit, right?

@weswigham weswigham merged commit 09a21ce into microsoft:master May 29, 2021
PR Backlog automation moved this from Needs merge to Done May 29, 2021
DanielRosenwasser pushed a commit that referenced this pull request Jun 16, 2021
@typescript-bot @weswigham
Cherry-pick PR #44126 into release-4.3 (#44189)
89a171e 
Component commits:
99110e9 Consider inferences between mapped type templates lower priority

Co-authored-by: Wesley Wigham <t-weswig@microsoft.com>
@Andarist Andarist mentioned this pull request Apr 25, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sandersn sandersn sandersn left review comments

@DanielRosenwasser DanielRosenwasser DanielRosenwasser approved these changes

@ahejlsberg ahejlsberg Awaiting requested review from ahejlsberg

Assignees

@weswigham weswigham

Labels
Author: Team For Milestone Bug PRs that fix a bug with a specific milestone
Projects
PR Backlog
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Regression: Type using conditional and mapped types is no longer assignable in 4.3
4 participants
@weswigham @typescript-bot @DanielRosenwasser @sandersn