Skip to content

Design Meeting Notes, 2023-03-01 #53057

Closed
@RyanCavanaugh

Description

@RyanCavanaugh
  • Check for strict subtypes and then regular subtypes in getNarrowedType #52984 - check for strict subtyping in UDTG narrowing
    • Origin: Break in lodash's isArrayLike Improvement to getNarrowedType changes lodash's isArray #52827
    • Problems occur when the asserted type and the tested type are subtypes of each other
      • This invariably leads to problems
    • Strict subtyping is more strict and prefers the type with the index signature
      • This is what we use for subtype reduction, since it is more directional (ideally produces no ties)
      • Without this, we fall back to Type ID, which is not stable
    • User-defined type guards (UDTG) were using non-strict subtyping
      • Some code depends on a non-strict subtype narrowing
      • But we want to prefer to pick the asserted type as much as possible?
    • This also creates CFA problems since the join point at the end of the if won't reconstruct the original type (!)
    • How to fix?
      • When UDTGing, narrow by strict subtyping, if that works (produces at least one type)
      • If it doesn't, fall back to nonstrict subtyping
    • This doesn't fix everything but attempts to make it "better" broke extant reasonable code
      • Read the PR notes
      • Only "broken" examples left at this point would basically have to be constructed to hit this case
  • Improve overload and generic signature inference: Inference alternatives and linked inferences #52944 - Improve overload and generic signature inference: Inference alternatives and linked inferences
    • Why?
      • Holy Grail: make JSX elements get checked like normal function calls
      • Why doesn't that work today?
        • Generic inference in the presence of overloads is very limited
          • Note that this is about a call to an unoverloaded function involving a parameter whose type has overloads
        • JSX inference is smarter
    • What does the PR do instead?
      • Try each of the signatures
      • Do inference to free type parameters; makes id-like functions work
      • This is effectively a 2-pass unification
    • Previous attempts?
    • Performance implications?
      • 10% hit on material-ui
      • Other projects largely unaffected
    • Other effects
      • Do pipe-like functions unify much better?
        • Ya, check linked issues in PR
    • What happens if multiple signatures match?
      • We pick the last one that worked, per compat since we previously only consulted the last one
      • Totally opposite to the existing overload rules 🙃
      • Doesn't yield the best results, unsurprisingly
      • What are the breaks?
    • What about contextually-sensitive expressions?
      • Still broken per our non-hierarchical node-to-type cache approach
      • Can we defer contextually-sensitive expressions like we do elsewhere (object literal properties)?
        • Seems equally hard as just implementing a cache layer per inference context TBH?
        • Error reporting is still difficult; would need to enforce laziness
        • Speculative implementation was based on prototypical inheritance, but we're not using objects as much these days
        • Pyright has this FWIW
        • This PR is instructive as to where cache acceptance should happen here, if we ever did it
    • Going back to motivation - what doesn't work without this?
      • JSX: (example needs to be provided...)
      • If we have this, we can switch to JSX factory function checking
      • Long-term goal: Deprecate JSX namespace as much as possible
      • Near-term problem: Promise<JSX> is coming
        • Want to stop digging a hole we don't want to be there
      • Is this the only remaining problem?
        • Unclear until we try it in earnest, probably
        • Regardless, it makes sense to go one (good) step at a time
      • If we do this, can we remove old JSX logic eventually?
        • Seems plausible
    • What are next steps?
      • Try to reduce material-ui perf hit?
      • Validate long-term JSX plan with the React .d.ts maintainer
      • Evaluate breaks from picking the top overload
      • Try to fix the comment posted by seansfkelley

Metadata

Metadata

Assignees

No one assigned

    Labels

    Design NotesNotes from our design meetings

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions