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, 8/9/2023 #55325

Closed
DanielRosenwasser opened this issue Aug 9, 2023 · 0 comments
Closed

Design Meeting Notes, 8/9/2023 #55325

DanielRosenwasser opened this issue Aug 9, 2023 · 0 comments
Labels
Design Notes Notes from our design meetings

Comments

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Aug 9, 2023

Control Flow Analysis for Symbol.hasInstance Properties

#55052

  • instanceof only really looks at classes, or things that kinda look like classes
    • Looks for a construct signature, maybe a prototype
  • That's not actually how ECMAScript works.
  • Since ES2015, instanceof is an indirect operation that routes through a [Symbol.hasInstance] method.
  • In engine262, a JS engine written in JS, they re-authored it to TypeScript.
    • They were using instanceof for this.
  • Created an abstract class that throws on construction, but overrides instanceof to work properly.
  • Why didn't they just use classes in the first place?
    • Not really clear.
  • Not specific to engine262, VS Code also configures a hasInstance for its API.
  • So this PR makes it so that anything with a hasInstance can be used on the right side of an instanceof.
    • If the [Symbol.hasInstance] is a type predicate function (has an (x: SomeType): x is SomeOtherType-like signature), then it narrows.
  • Something interesting in this PR - you can use this to restrict what values can be provided on the left side.
  • You can also allow allow more types to be permitted. For example, no restriction that the left side must be an object - can permit 123 instanceof Foo
    • This could come in handy for pattern matching in the future?
  • Is that a good idea? It's gotta be some concept other than instanceof for matching.
  • Would tie into the protocols proposal.
  • Why do it this way?
    • Argument about readability.
  • Questions about performance.
    • How slow is it for an engine to optimize instanceof over a direct function call?
    • How slow is it for our type-checker to start doing this?
  • How is narrowing affected? instanceof is special today, right?
    • We do a special check for derivedness.
    • If you have a custom [Symbol.hasInstance], instanceof narrowing should take on the same semantics as type predicate narrowing.
  • If we do this, we want go-to-definition on instanceof to jump you to the predicate.
    • Same with quick-info.
  • Seems easy to accidentally define an instance-level [Symbol.hasInstance]. Can we add an error message?
    • No! It's never going to be an error to instanceof from the class.
    • Probably something a linter should enforce.
  • If we could stop hasInstance from existing, we probably would. Maybe it was necessary for Proxy objects. Ideally though, pattern matching would be the "pluggable" behavior and instanceof was left alone.
  • Conclusion: begrudgingly yes - it's a gap in what JS itself does. But must address the aforementioned feedback.

Consolidate tsserverlibrary.js and typescript.js

#55273

  • People need a ts.server.ProjectService.
    • Can just open a file, ask questions about files without explicitly constructing a project and whatever. Good for sharing files between projects and managing the lifetime etc.
  • To get it, people refer to tsserverlibrary.js - but then they will occasionally mix values between typescript.js and tsserverlibrary.js.
  • TypeScript-ESLint has specifically run into issues here often.
  • To move the project service into typescript.js, you have to duplicate so much - might as well redirect one to the other.
  • So idea: typescript.js also gets the fundamental innards of the services layer as well as the TSServer library.
  • Why weren't they all just in the same bundle before?
    • Artificial boundaries? Uncertainty about stability of TSServer? Visual Studio not consuming TSServer at all back then? All of this probably.
  • ts.server.ProjectService is itself very useful and people don't know about it. People get the impression that if you use tsserverlibrary.js the only useful thing you get is tsserver. But if, say, you wanted to create your own special interactive tsc.js, you can possibly use ts.server.ProjectService (?)
  • Also: tsserver.js itself (the executable Node implementation) is only 1000 lines of code beyond tsserverlibrary.js. Just includes the Node server implementation. That would be another step in simplifying what we ship if we desired.
    • And lots of people probably would probably find that useful.
    • But not proposing that today.
  • Aside: would like to also expose executeCommandLine etc.
  • Also, we started exporting the server layer from the services layer.
  • Conclusion: do it for 5.3.
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