-
Notifications
You must be signed in to change notification settings - Fork 133
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(lint): Do not directly call object builtins #344
Conversation
@@ -1140,7 +1140,7 @@ AmplitudeClient.prototype.identify = function (identify_obj, opt_callback) { | |||
} | |||
|
|||
// if identify input is a proxied object created by the async loading snippet, convert it into an identify object | |||
if (type(identify_obj) === 'object' && identify_obj.hasOwnProperty('_q')) { | |||
if (Object.prototype.hasOwnProperty.call(identify_obj, '_q')) { |
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.
trying to call this on a non-object-like will fail, so I got to remove a check here :)
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.
LGTM!
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.
Looks good, one thought: would it make any sense to make a util fn that wraps Object.prototype.hasOwnProperty.call
?
## [7.4.2](v7.4.1...v7.4.2) (2021-02-11) ### Bug Fixes * **lint:** Do not directly call object builtins ([#344](#344)) ([14fc693](14fc693))
Summary
We are heavy users of the Object prototype
hasOwnProperty
. In general, these should not be called directly - especially on user-created inputs. Allows a lint override to be removed.Did not add it to the
tests/browsers
dir because I'm not linting sinon or chai 🤷Checklist