Support custom 'Symbol.hasInstance' methods when narrowing 'instanceof' #55052
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…wing 'instanceof'
typescript-bot
added
Author: Team
For Uncommitted Bug
|
Looks like you're introducing a change to the public API surface area. If this includes breaking changes, please document them on our wiki's API Breaking Changes page. Also, please make sure @DanielRosenwasser and @RyanCavanaugh are aware of the changes, just as a heads up. |
rbuckton
changed the title
|
@typescript-bot pack this |
|
Hey @gabritto, I've packed this into an installable tgz. You can install it for testing by referencing it in your and then running There is also a playground for this build and an npm module you can use via |
gabritto
reviewed
Jul 18, 2023
|
@typescript-bot perf test |
|
Heya @rbuckton, I've started to run the diff-based user code test suite on this PR at 6a4f838. You can monitor the build here. Update: The results are in! |
|
Heya @rbuckton, I've started to run the perf test suite on this PR at 6a4f838. You can monitor the build here. Update: The results are in! |
|
Heya @rbuckton, I've started to run the parallelized Definitely Typed test suite on this PR at 6a4f838. You can monitor the build here. Update: The results are in! |
gabritto
approved these changes
Jul 18, 2023
|
@rbuckton Here are the results of running the user test suite comparing There were infrastructure failures potentially unrelated to your change:
Otherwise... Something interesting changed - please have a look. Details
|
|
@rbuckton Here they are:
CompilerComparison Report - main..55052
System
Hosts
Scenarios
TSServerComparison Report - main..55052
System
Hosts
Scenarios
StartupComparison Report - main..55052
System
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Hey @rbuckton, the results of running the DT tests are ready. |
|
@typescript-bot pack this |
|
Hey @rbuckton, I've packed this into an installable tgz. You can install it for testing by referencing it in your and then running There is also a playground for this build and an npm module you can use via |
|
Leaving a random comment to test the new perf infra on this PR. Update: The results are in! |
sandersn
requested changes
Sep 19, 2023
tests/baselines/reference/typeGuardsWithInstanceOfBySymbolHasInstance.types
Show resolved
Hide resolved
tests/baselines/reference/typeGuardsWithInstanceOfBySymbolHasInstance.types
Show resolved
Hide resolved
sandersn
reviewed
Sep 19, 2023
sandersn
approved these changes
Sep 19, 2023
ahejlsberg
reviewed
Sep 22, 2023
rbuckton
commented
Sep 22, 2023
|
The most recent commits address feedback from an offline discussion with @ahejlsberg. @ahejlsberg, can you take another look? |
ahejlsberg
approved these changes
Sep 28, 2023
|
@typescript-bot cherry-pick this to release-5.3 |
|
Hey @rbuckton, I couldn't open a PR with the cherry-pick. (You can check the log here). You may need to squash and pick this PR into release-5.3 manually. |
typescript-bot
pushed a commit
to typescript-bot/TypeScript
that referenced
this pull request
Sep 29, 2023
Component commits: 69f59da [WIP] Support custom 'Symbol.hasInstance' methods when checking/narrowing 'instanceof' 161d1f1 Add tests for instanceof and narrowing 6a4f838 Accept baseline, fix lint af6bb55 Check derived types when using type predicates with instanceof/hasInstance 6a83254 Small tweaks, lint fixes, and baseline updates 511c955 Add go-to-definition support on 'instanceof' keyword 9e3adb4 Merge branch 'main' into instanceof-Symbol.hasInstance 4b0eafc Fix format befa293 Address PR feedback 559047e Comment cleanup 8af777e Switch synthetic call to use use 'resolveSignature' flow f3e94f0 Merge branch 'main' into instanceof-Symbol.hasInstance 1d90af1 Run formatting b65f9bc Merge branch 'main' into instanceof-Symbol.hasInstance 0554f56 Remove branch for 'instanceof' error message reporting
|
I have no idea why the pick failed; I was going to say that it failed a the push stage but it got pushed above: typescript-bot@97bf30b Probably some weird GH outage :| (I am working on replacing this pick task anyway.) |
|
It doesn't matter too much in this case. Daniel said he'd just update from |
This was referenced Jul 31, 2024
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This makes our
instanceofnarrowing more closely align to Steps 2-3 of the InstanceofOperator algorithm in the ECMA-262 specification by allowing any object type in the right-hand side ofinstanceofas long as it has a valid[Symbol.hasInstance]method. If such a method exists and returns a type predicate, that predicate can then be used when narrowinginstanceof.For a
[Symbol.hasInstance]method to be considered duringinstanceofnarrowing, it must have a type predicate as its return type.In addition, when the left-hand side of
instanceofis considered as an argument to[Symbol.hasInstance], it must be assignable to the first parameter. This does not, however, weaken our current requirement that the left-hand side be an object type (or a union containing an object type), despite the fact this is not currently a requirement of ECMAScript.This is intended to address rather complicated workarounds employed by, for example,
engine262which uses[Symbol.hasInstance]as a runtime type guard, but needed to introduce a non-creatable abstract class so that TypeScript would understand the type guard: https://github.com/engine262/engine262/blob/435e533f8bb2f66961e3c16fe67150a0bba745bb/src/completion.mts#L70Fixes #17360
Fixes #39064
Related #32080
Related #52670
NOTE: This will need to wait for 5.3 at the earliest.