-
Notifications
You must be signed in to change notification settings - Fork 13.1k
Closed
Labels
Design NotesNotes from our design meetingsNotes from our design meetings
Description
Control Flow Analysis for Symbol.hasInstance Properties
instanceofonly really looks at classes, or things that kinda look like classes- Looks for a construct signature, maybe a
prototype
- Looks for a construct signature, maybe a
- That's not actually how ECMAScript works.
- Since ES2015,
instanceofis 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
instanceoffor this.
- They were using
- Created an
abstractclass that throws on construction, but overridesinstanceofto 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
hasInstancefor its API. - So this PR makes it so that anything with a
hasInstancecan be used on the right side of aninstanceof.- If the
[Symbol.hasInstance]is a type predicate function (has an(x: SomeType): x is SomeOtherType-like signature), then it narrows.
- If the
- 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
instanceoffor matching. - Would tie into the
protocolsproposal. - Why do it this way?
- Argument about readability.
- Questions about performance.
- How slow is it for an engine to optimize
instanceofover a direct function call? - How slow is it for our type-checker to start doing this?
- How slow is it for an engine to optimize
- How is narrowing affected?
instanceofis special today, right?- We do a special check for derivedness.
- If you have a custom
[Symbol.hasInstance],instanceofnarrowing should take on the same semantics as type predicate narrowing.
- If we do this, we want go-to-definition on
instanceofto 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
instanceoffrom the class. - Probably something a linter should enforce.
- No! It's never going to be an error to
- If we could stop
hasInstancefrom existing, we probably would. Maybe it was necessary forProxyobjects. Ideally though, pattern matching would be the "pluggable" behavior andinstanceofwas 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
- 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 betweentypescript.jsandtsserverlibrary.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.jsalso 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.ProjectServiceis itself very useful and people don't know about it. People get the impression that if you usetsserverlibrary.jsthe only useful thing you get is tsserver. But if, say, you wanted to create your own special interactivetsc.js, you can possibly usets.server.ProjectService(?)- Also:
tsserver.jsitself (the executable Node implementation) is only 1000 lines of code beyondtsserverlibrary.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
executeCommandLineetc. - Also, we started exporting the server layer from the services layer.
- Conclusion: do it for 5.3.
jakebailey, jeswinsimon, uhyo, Zzzen and JoshuaKGoldberg
Metadata
Metadata
Assignees
Labels
Design NotesNotes from our design meetingsNotes from our design meetings