-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Simplify type checking for objects #11831
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@msujew should we maybe update packages/core/src/common/types.ts to include similar methods present in vscode (copying them over)?
As trivial as the checks might be I think unit-testing them might be good too.
edddeff
to
5a852e6
Compare
I pushed a commit with an alternative writing: The reasoning is that the function names should better describe what it does ( import * as types from '@theia/core/lib/common/types';
// ...
types.isObject(...); |
Cleanup our `packages/core/src/common/types.ts` file.
9edcebb
to
0fe602c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like these new functions as they greatly help when dealing with unknown
-typed variables!
@msujew feel free to merge if you agree with the changes suggested in the added commit, otherwise drop it before. |
@paul-marechal Thanks, I didn't have time recently to update this PR :) Your changes look good to me 👍 |
What it does
A follow up on #11490 (cc @paul-marechal)
Introduces an
Is
namespace with helpful methods to identify types. Uses the newly introduced methods to simplify type checking for objects. I didn't use theIs.string
,Is.number
etc functions as that would make the PR even larger and I wasn't sure whether it would make sense to use the functions that way anyway.For some reason, this change required an additional call to
reflect-metadata
in the plugin host. Otherwise it would crash as theContributionFilterRegistryImpl
would try to annotate itself without theReflect.hasOwnMetadata
function available.How to test
Nothing should've changed
Review checklist
Reminder for reviewers