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

Temporarily revert unconstrained type parameter strictness in TS 4.7 #48923

Merged
merged 2 commits into from
May 2, 2022

Conversation

DanielRosenwasser
Copy link
Member

@DanielRosenwasser DanielRosenwasser commented May 2, 2022

This PR reverts #48366.

We've seen that the change to disallow a generic T to be assignable to {} are particularly disruptive to existing codebases. Some examples of where this goes awry:

  • Incompatible constraints when assigning unconstrained generics to {}
  • Incompatible constraints when assigning unconstrained generics to object
  • Incompatible constraints when assigning unconstrained generics to { all?: string, optional?: number, objectTypes?: boolean }
  • Incompatible values because not all generics are narrowed

We've heard about or saw examples of these on Definitely Typed, Visual Studio Code, and internal codebases at Microsoft, Google, and Bloomberg. In particular, @amcasey and I were surprised to find out about the all-optional-objects break, and it sounds like @dragomirtitian and @robpalme might be dealing with that one heavily.

Definitely Typed has already been updated which should help a bit; however, I think that we have other avenues to make this change more approachable.

First, we could break the breakages across versions to make it easier to upgrade. We could narrow the previous exception on unconstrained generics so that an unconstrained T is assignable to strictly {} and object, or strictly not {} and object. This would allow users to only focus on these cases, or the all-optional-object-type break.

Additionally, @ahejlsberg has been thinking about alternative ways to do narrowing on generics. We would pull that in now, but it's not well-understood in the context of the broader ecosystem. That will have to wait until 4.8.

If we bring this in, my advice for people who may have a pending upgrade is to keep their changes in place. The new constraint enforcement is still more correct, and we'd like to bring it in to a newer version of TS anyway. (CC @frigus02)

@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels May 2, 2022
@DanielRosenwasser DanielRosenwasser changed the title Temporarily revert unconstrained type parameter strictness Temporarily revert unconstrained type parameter strictness in TS 4.7 May 2, 2022
@weswigham
Copy link
Member

weswigham commented May 2, 2022

In particular, @amcasey and I were surprised to find out about the all-optional-objects break,

This was exactly one of the problems it was meant to solve and was a good portion of the issues we fixed in DT even two years ago. These cases are the particularly bad places that we really really should catch, since they mask a place where we may be assigning a null or undefined value to a non-nullable type, the core value proposition. It is very clearly unsafe to assign a T with no constraint to an object with exclusively optional properties (Or pretty much any concrete object and not a union that includes null and undefined). :P

@ahejlsberg
Copy link
Member

I think we all agree that we want these changes at some point, but it has also become clear that they cause a lot of breakage. Moving to 4.8 gives us more time to consider possible mitigations, opt-outs through compiler switches, and/or improvements to our narrowing logic.

@DanielRosenwasser DanielRosenwasser merged commit 3b8b207 into main May 2, 2022
@DanielRosenwasser DanielRosenwasser deleted the revertUnconstrainedTypeParameterStrictness branch May 2, 2022 22:49
@weswigham
Copy link
Member

Q: There a reason this PR wasn't just made against the 4.7 branch and merged post-RC cut? As is we'll be churning the nightly's behavior when we've acknowledged that we do want this behavior, just potentially alongside others?

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.

5 participants