-
Notifications
You must be signed in to change notification settings - Fork 60
Add custom type assertions for goog.is* functions. #886
Conversation
I am torn here, on one hand this is a good change for people that are still using isBoolean -> which TypeScript has no problem of type narrowing from. I don't recommend anyone keep using |
Why not? We find the named functions easier to read than the actual implementation. Also on a related note, I just discovered that goog.reflect.object also needs a custom type:
The autogenerated one is completely useless. It requires casts to |
Let's get second opinion from @shicks. I think not having this special case will nudge people towards inlining |
Are you aware of anything like refasterjs for typescript to help migrate our existing code? |
emit("(val : any ) : val is any[] "); | ||
return true; | ||
} | ||
if (propName.equals("isBoolean")) { |
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'm actively working on deprecating and (eventually) deleting all of these. I'd rather not be encouraging their use.
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.
In particular, I've got an LSC in progress to replace isString
, isBoolean
, isNumber
, isDef
, isNull
, and isDefAndNotNull
for all internal JS and TS code in one fell swoop.
I may look into doing the other three as well, and possible goog.typeof
if I can figure out how to do it safely.
@ribrdb about global refactorings, we have been talking about the different options, but for now we just use TSLint with its autofixer API. Also, for these APIs something as simple as 'sed' might suffice. |
Closing this as we are moving away from 'goog.is*' functions. I think something similar will be interesting to do once Type assertions land in 3.7 for Closure's assert methods. |
Fixes #710