Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(lint): Do not directly call object builtins #344

Merged
merged 2 commits into from
Jan 13, 2021
Merged

Conversation

kelvin-lu
Copy link
Contributor

@kelvin-lu kelvin-lu commented Jan 12, 2021

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

  • Does your PR title have the correct title format?
  • Does your PR have a breaking change?:

@@ -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')) {
Copy link
Contributor Author

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 :)

Copy link
Contributor

@jooohhn jooohhn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Contributor

@kelsonpw kelsonpw left a 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?

@jooohhn jooohhn changed the base branch from master to main January 13, 2021 00:56
@kelvin-lu kelvin-lu merged commit 14fc693 into main Jan 13, 2021
@kelvin-lu kelvin-lu deleted the 273/object-prototype branch January 13, 2021 08:22
github-actions bot pushed a commit that referenced this pull request Feb 11, 2021
## [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))
@jooohhn jooohhn mentioned this pull request Feb 22, 2021
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants