-
Notifications
You must be signed in to change notification settings - Fork 3k
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
fix(isObservable): Fix throwing error when testing isObservable(null) #3688
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.
This isn't quite right. We don't need to check to see if it's a function or object, we just needed to group the checks after the "falsiness" check.
return obj && (obj instanceof Observable || (typeof obj.lift === 'function' && typeof obj.subscribe === 'function'))
@benlesh Above suggestion gave me |
src/internal/util/isObservable.ts
Outdated
@@ -1,10 +1,12 @@ | |||
import { Observable } from '../Observable'; | |||
import { ObservableInput } from '../types'; | |||
import { isObject } from './isObject'; | |||
import { isFunction } from './isFunction'; |
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.
Remove unused imports.
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.
Good catch, updated.
thanks @bbonnet! |
Description:
Fix
isObservable
to test to make sure the test subject is a function or object before attempting to access properties on it.Related issue (if exists):
#3687