You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains 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
The reason will be displayed to describe this comment to others. Learn more.
I think I'd just leave the boolean type. The method is not checking if o is an object, but if it's a plain object, I don't think you can reproduce that correctly with TypeScript
6183667
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.
This change is causing a TypeScript error now (TypeScript 3.9.7):
Build: https://github.com/octokit/endpoint.js/pull/170/checks?check_run_id=897150799#step:5:15
I wonder what the motivation for that change was?
I'm by no means a TypeScript expert, but from what I've heard, the
object
type should be avoided6183667
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.
ping @lifeiscontent ☝🏼
6183667
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.
@gr2m Do you have an idea how to implement proper object assert? I'm not typescript user so can't judge.
6183667
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 think I'd just leave the
boolean
type. The method is not checking ifo
is an object, but if it's a plain object, I don't think you can reproduce that correctly with TypeScript6183667
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.
Published 4.1.1 with revert. Thanks for pointing to this.
6183667
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.
Thanks a lot!