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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: remove ambiguous "this" in favor of passed event and currentTarget #1291

Closed
wants to merge 1 commit into from

Conversation

jsr6720
Copy link

@jsr6720 jsr6720 commented Jun 10, 2016

"this" can be ambiguous in JS. So let's use the event that's already passed to the function.

@dylans
Copy link
Contributor

dylans commented Jun 11, 2016

Not sure I agree with this change, as the on API makes it clear that this always refers to the event target within the callback function.

@jsr6720
Copy link
Author

jsr6720 commented Jun 13, 2016

@dylans agreed. Turns out we have some internal code that is hijacking the event. Though I was curious why the event is not used? Is this the same pattern for TypeScript?

@dylans
Copy link
Contributor

dylans commented Jun 13, 2016

We generally only include the event object as a parameter when we use it... saves an object in memory basically. It's still there as the first argument though, so you could read it from arguments[0] for example.

@jsr6720
Copy link
Author

jsr6720 commented Jun 20, 2016

Thanks for the feedback. We're going to fix the conflict internally.

@jsr6720 jsr6720 closed this Jun 20, 2016
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

2 participants