-
Notifications
You must be signed in to change notification settings - Fork 134
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # 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