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

Design Meeting Notes, 2/14/2020 #36877

Closed
DanielRosenwasser opened this issue Feb 19, 2020 · 0 comments
Closed

Design Meeting Notes, 2/14/2020 #36877

DanielRosenwasser opened this issue Feb 19, 2020 · 0 comments
Labels
Design Notes Notes from our design meetings

Comments

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Feb 19, 2020

Touched-up edits from me on @uniqueiniquity's notes

Discriminated Narrowing with typeof

#32399

  • Today we do checks to narrow types depending on specific checks on literal types
  • But using typeof doesn't narrow in the same way when checking against primitive types
  • in the checking process order, we check for discriminated properties very early
  • Brian's example would work easier than the original example
  • Once there is an object in a property, that makes it no longer possibly discriminating
    • null/undefined are different
  • two separate features:
    • discriminate inside of typeof test (make typeof apply discriminate narrowing)
      • this is the one azure sdk wants
    • discriminate when something is an object (as opposed to is not null or undefined)
  • We should do the former, probably not the latter

Non-null assertions ! with Optional Chaining ?.

#36031

  • Revisiting issue from a month ago
  • The bang breaks the chain, thereby causing unexpected accesses on undefined
  • After exploring a solution, the problem is that a?.b.c! can still be undefined
  • Option A: Can we special case that example?
  • Option B: Can we say that the bang is part of a chain only when it's in the middle?
  • C# has similar struggles with this example
  • Our parsing strategy caused this issue - technically postfix bang shouldn't be allowed to be mixed
    • let's keep it allowed, too awkward to disallow
  • why would you have a trailing bang?
    • defend against incorrect library types, so we do care
  • Option B is definitely preferred - seems straightforward and understandable
  • concern about breaking change if we do option B
  • Option C: we could require parens
  • How bad is this problem?
    • Unintuitive for users
    • It has 4 upvotes
  • Conclusion: Option B!
  • Do we want it for 3.8.2? No, wait for 3.9.

Can appending .js in Auto-Imports make sense?

#36725

  • An extension (e.g. .js) is required in Node ESM and Browser ESM for ESM imports
  • Add another option? Actually, we already have it...
    • importModuleSpecifierEnding for editor options
  • should it just be determined by emit? no, might be using a bundler
  • Do we need to support export maps to do this correctly?
    • as part of full Node ESM support, but not piecemeal
  • VS Code just wants the option
  • Also we've gotten requests for it to be an error to leave off the extension, but that's a separate issue
  • Now that browsers support import maps, classic resolution is not the best mode anymore, might need a third that supersedes these fixes
  • Should we give the option to VS Code now or say we'll get this done as part of future resolver work?
    • worried that customers don't use what this is for, but we should really just have it
  • Let's ship the auto-import option for 3.9, it's already there.

Making Impossible Intersections of Discriminated Unions Produce never

#36696

  • Our type rewriting rules often create types that don't make sense

  • Originally didn't resolve this because we thought it was expensive and would cause circularities

  • Now, once we're about resolve the members of the union, we reduce, instead of reducing up front

  • you can view this as detecting the intersection version of discriminants

  • note that if one has an explicitly designated never type, we allow this

    type A = { kind: 'a', foo: string};
    type B = { kind: 'b', foo: number};
    type C = { kind: 'c', foo: number };
    declare let x: A | (B & C);
    let a: A = x; // previously error, now the same
  • Originally motivated because we can't run the compiler with unions in our perf suites because of Empty intersection reduction #31838 making types more inclusive caused Node's parent property to be massive

  • Now by reducing when the parent property is accessed, it greatly helped the problem

  • 10% speedup for material-ui, will help styled-components as well, and React's ref property

  • Key insight is that the only way to solve this problem is to not do it when constructing the intersection, but instead defer until a member is resolved

  • This is a breaking change: we will explore more members than we have before

  • We found two bugs in the current compiler, found bugs in DT

  • Not going to detect doubly nested discriminants at the outer level, so that won't break

    declare let xx: B & C;
    xx.foo // today you get number, with this change its an error
  • Willing to take the breaking change nature of this because we found errors and the perf benefits are a clear win

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Notes Notes from our design meetings
Projects
None yet
Development

No branches or pull requests

2 participants