Repository navigation
Design Meeting Notes, 1/18/2023 #52296
Copy link
Copy link
Closed
Labels
Design NotesNotes from our design meetingsNotes from our design meetings
Description
newLine and forceConsistentFileNames Defaults
newLinetolf?- Didn't get to discuss it thoroughly.
- Change default of
forceConsistentFileNamestotrue?- Catch issues between Windows/Mac/Linux ahead of pushing out to CI.
- We think it's good to turn on.
- We all feel okay about the fact that it doesn't "do what people think", right?
- "What?"
- It doesn't check whether the paths match what's on disk, it just checks whether the paths are consistent across the project.
- Deprecate it?
- Prefer not to.
- Conclusion
--newLine lfby default.--forceConsistentFileNames trueby default.
catch Clause Variables
// @useUnknownInCatchVariables: false
try {} catch ({ e }) {}
// ^?
try {} catch ({ e }: any) {}
// ^?
try {} catch ({ e }: unknown) {}
// ^?
try {} catch ({ x: { e } }) {}
// ^?
try {} catch ({ x: { e } }: any) {}
// ^?
try {} catch ({ x: { e } }: unknown) {}
// ^?- Toggle
useUnknownInCatchVariablesbetweentrueandfalse. - When
true, the annotations are ignored, and destructured declarations are permitted andunknown. - Make catch clause checking consistent with variable declarations #52240 fix this.
- Breaking change, but only found buggy code under
useUnknownInCatchVariables.- We think this is good for 5.0.
- Also: Allow arbitrary catch clause type annotations #52280 allows arbitrary type annotations on catch variables - addresses Allow type annotation on catch clause variable #20024.
- But we're not doing that, right?
- No, blatantly unsafe.
- Well then we should tell people that we're not doing it?
- Context matters - we think pattern matching progressing in TC39 is going to enable a safe version of this pattern.
- We want to defer to that.
- Conclusion
- We're not doing Allow arbitrary catch clause type annotations #52280.
- We're doing Make catch clause checking consistent with variable declarations #52240.
Comparison Operators
- Some obvious but questionable things that popped up.
Number < numbernow disallowedstring | number < numbernow disallowedunknown < numbernow disallowed
- Some stuff that fails due to control flow limitations across functions.
- What about objects that coerce to
number(e.g.Date)?- Why don't we look at
valueOf?- Or
[Symbol.toPrimitive]? - Here we go...
- Or
- Why don't we look at
- Conclusion: Feels like this PR is a good start. Want to bring this in for 5.0.
- Watch for 5.0 Beta feedback.
JSON.stringify
- This thing broke some code.
- Technically
stringifycan returnundefined.- But technically also wrong because a
Functioncan have atoJSONmethod.
- But technically also wrong because a
- There's a world where we could possibly keep this as-is if we reordered the signatures so that
neverworks.- Overload resolution goes through a subtype pass.
- But why are we adding this overload? Who ends up in a situation like this?
- Doesn't even catch mistakes like
(() => string) | stringtoday. - Only contrived cases are caught.
- Doesn't even catch mistakes like
- The idea that everyone has to suffer for
stringifybecause of these contrived-but-uncommon cases feels unreasonable. - Could ship an open-ended union?
- Could also just tell people to interface-merge.
- Conclusion:
- Revert this, tell people to interface-merge on
JSONwith something that returnsundefined.
- Revert this, tell people to interface-merge on
Picking Between Types from Type Predicates and Original Types
- Made a change a while back to pick the types from type predicates/type assertions.
- Similar behavior with
instanceoftoo. - Means that the original type does not survive at joining points of CFA.
- Idea
- In the true branch, we will still prefer the type predicate type.
- In the false branch, we will now preserve the original type so that they join back at a CFA merge node.
- So what do we now do about mutual subtypes?
- For example,
unknownandanyare mutual subtypes. - Today we choose the type with the lowest ID.
- Pretty unfortunate - these have measurable effects.
- So we are making
anya strict supertype ofunknown.- Behavior directly in unions is the same.
- Where else does this appear?
-
When the type IDs are not compared directly.
-
So for e.g.
Array<any>vs.Array<unknown>- butArrayis not special.type Box<T> = { readonly value: T }; declare let anyBox: Box<any>; declare let unknownBox: Box<unknown>; let boxes1 = [anyBox, unknownBox]; // ^? let boxes2 = [unknownBox, anyBox]; // ^?
-
Now you always end up with
Box<any>[].
-
- Similar constraints with
{}and any non-empty object type.
- For example,
- What other effects does this have?
-
The type of an assignment expression is always the right side:
function f(obj: { a?: string }) { (obj = {}).a; // allowed? }
-
Hmm...
function f(a: { x?: number }, b: { x?: string}) { a = b = {}; // allowed? // Equivalent to `a = (b = {})` which naively looks like `b = {}; a = {}` from a type perspective. a.x = 42; b.x?.toUpperCase(); }
- Fresh
{}object type has "inverted" subtype behavior.- It is, but on the assignment we widen.
- Do we have to widen? Could choose not to.
- Feels like for the
b = {}, we have to propagate out some information, or further narrow based on the left side(?)
- Fresh
-
(...args: any[]) => anyshould be a super-type of all functions?- But what about
(...args: never[]) => unknown?- Not specially understood today by TS internals.
- But what about
-
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
Design NotesNotes from our design meetingsNotes from our design meetings
Activity
fatcerberus commentedon Jan 19, 2023
Isn’t the “any-like” function type
(...args: any) => any?Andarist commentedon Jan 19, 2023
IIRC - yes. But the fact that it's not
(...args: any[]) => anycreates some issues, like this one. I was experimenting with changing the effective rest of thisAnyFunctiontypeany[](and notany) here but IIRC that in turn created some issues with tuple types asany[]ain't assignable to all tuple types.Andarist commentedon Jan 19, 2023
While some of those are contrived and uncommon - this isn't quite that (in my opinion):
I had a production issue recently cause I accidentally passed an optional param/property to a stringifying function and somewhere down the road I expected to have a guaranteed
stringfatcerberus commentedon Jan 19, 2023
Also IIRC
(...args: never) => unknownis understood by TS as the function top type.never[]is problematic because then it’s legal to call it with no arguments.DanielRosenwasser commentedon Jan 19, 2023
Mateusz Burzyński (@Andarist) I tried to capture it in the notes, but the new overload doesn't model that type correctly anyway -
object | undefinedwill fall into theanycase and provide you withstring.DanielRosenwasser commentedon Jan 19, 2023
Anders Hejlsberg (@ahejlsberg)
Also - TIL that
neveris a valid rest parameter type.RyanCavanaugh commentedon Jan 20, 2023
The thing that got us super derailed on the JSON.stringify thing was that people have the entirely reasonable assumption that if they pass something that isn't
undefined, especially something that isobject, then it "definitely won't" returnundefined. But this isn't true -- functions areobject(there areinstanceof Object, they have all of Object's properties, etc), so given any value in TypeScript, we don't have any way to guarantee that it isn't actually referencing something that is secretly a function (or, more degenerately, has atoJSONmethod that returnsundefined). And passing a function toJSON.stringifywill returnundefined, unless that function has a customtoJSONmethod. This isn't even particularly theoretical, since class constructors are also functions, yet can easily have static methods that satisfy object types.Thus while you could write a conditional type to distribute over the possible inputs and say that it only possibly returns
undefinedif one of the inputs is possiblyundefined... that type would also be wrong, in exactly the same failure mode that the type today is wrong.So we're basically stuck with two options:
undefined, which is obviously wrong and extremely annoying when you break prodstring | undefined, which is also obviously wrong and extremely annoying when in practice you will, 99% of the time, be able to construct a sound argument that none of the things I'm warning you about actually happen in this code, as far as anyone can tell, probablyRegardless, if you do want the alternate behavior, it's very easy to add an interface merge to your project and get the "always might be undefined" behavior. No commandline flags / custom lib / etc needed. Just hope that you don't overly habituate on writing
JSON.stringify(x)!and end up right back where you started 😉Andarist commentedon Jan 20, 2023
Right, I see the reasoning and I agree that adding
| undefinedwould be quite extremely annoying (but I also tend to think that the type system is here to "annoy" us 😉 ). The problem with the proposed solution (having to use interface merging) is that one has to opt into this - which for most projects would mean that they never opt into this or that they only opt into this after they break production.I understand though that this is a very hard/impossible choice to make here and there is no way to satisfy everybody.
Well, at least that surfaces the possibility to the writer and reader, it educates them about this problem. So this is a good thing - even if people start adding
!to eachJSON.stringifycall. It shifts the responsibility onto the dev.fatcerberus commentedon Jan 20, 2023
To a point. The design goals do include the caveat “strike a balance between correctness and productivity” for a reason. JS has no undefined behavior and soundness is explicitly not an end goal so it’s always a subjective judgment call what should be a compile-time error vs. not.
cherryblossom000 commentedon Jan 22, 2023
Bruce Pascoe (@fatcerberus) This code doesn’t cause any errors:
Playground
fatcerberus commentedon Jan 22, 2023
That looks like a bug.
[](the effective type ofargsin that call) is not assignable tonever. Compare that tonever[], for which an empty array is a legal value.cherryblossom000 commentedon Jan 22, 2023
Found the issue for that bug: #48840