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

Use cached noop function #33496

Merged
merged 5 commits into from
Apr 11, 2021
Merged

Use cached noop function #33496

merged 5 commits into from
Apr 11, 2021

Conversation

rohit2sharma95
Copy link
Collaborator

noop function returns a new function every time whenever it is called. So just use the noop function, not the value returned by it (So that the event handler can remove the same function that was attached before).

Ref: #33451 (comment)

@XhmikosR
Copy link
Member

I recall noticing this in the past but forgot to make a separate PR for sure. :) That being said, shouldn't our tests have caught this? I mean null !== noop(). The extra args are OK, though.

@XhmikosR XhmikosR requested a review from GeoSot March 30, 2021 12:50
@GeoSot
Copy link
Member

GeoSot commented Mar 30, 2021

@XhmikosR I suppose, you remember this
Do we still need this tweak @patrickhlauke ?

@XhmikosR
Copy link
Member

XhmikosR commented Apr 5, 2021

Yeah, that's another change :) But I still wonder why this wasn't caught :/

@rohit2sharma95
Copy link
Collaborator Author

Do we still need this tweak @patrickhlauke?

Not sure about that but that should be considered in another PR IMO (If that is not needed) @GeoSot. 🙂
So this PR is complete and will help #33466 to proceed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants