Skip to content

Event: Move .bind() and .delegate() to deprecated #2624

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.

Sign up for GitHub

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

Jump to bottom
Closed
dmethvin wants to merge 2 commits into from
Closed

Event: Move .bind() and .delegate() to deprecated #2624

dmethvin wants to merge 2 commits into from

Conversation

@dmethvin
Copy link
Member

@dmethvin dmethvin commented Oct 1, 2015

Fixes #2288

At the moment we don't have any tests at all for .bind() and .delegate(), it seems like we would at least want a sanity check for them. At the very least they should be tested in Migrate.

While I'm in here I'm wondering if we should deprecate anything else in the aliases.

dmethvin
dmethvin reviewed Oct 1, 2015
View reviewed changes
src/event/alias.js
Copy link
Member Author

@dmethvin dmethvin Oct 1, 2015

Choose a reason for hiding this comment

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

In case you haven't heard, I'm not a fan of this method.

Copy link
Member

@mgol mgol Oct 2, 2015

Choose a reason for hiding this comment

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

I agree, it's confusing. We could deprecate it in 3.0 & remove in 4.0 perhaps?

@mgol
Copy link
Member

mgol commented Oct 2, 2015

LGTM. Tests would be nice but since you're just moving code around it's not a blocker here.

BTW, we've had .bind()/.delegate() tests until I switched them to .on() in 8ca9f93 & 84a94ac (one of my first contributions to jQuery!). But it had to be done as we had too low coverage of .on()/.off() testing before that. :)

Should we duplicate a few basic .on() tests, creating .bind()/.delegate() versions?

@dmethvin
Copy link
Member Author

dmethvin commented Oct 2, 2015

I don't mind adding tests to this PR, just some basic sanity tests that are copies of what we already have but calling the deprecated methods. And we already have an empty test/unit/deprecated.js, yay!

@timmywil
Copy link
Member

timmywil commented Oct 2, 2015

In my opinion, anything more than

equal( jQuery.fn.on, jQuery.fn.bind, ... );

is just redundant.

@mgol
Copy link
Member

mgol commented Oct 2, 2015

@timmywil They're not equal, .bind() invokes .on(). That's why real tests are needed.

@timmywil
Copy link
Member

timmywil commented Oct 2, 2015

Of course. It's been a long time since I've used them. :P

@markelog
Copy link
Member

markelog commented Oct 12, 2015

Tests are added, are we good to go?

@dmethvin dmethvin mentioned this pull request Oct 12, 2015
Closed
@dmethvin
Copy link
Member Author

dmethvin commented Oct 12, 2015

Should be good to go. Just added the Migrate issue

@markelog
Copy link
Member

markelog commented Oct 12, 2015

Okay, merging

dmethvin added a commit that referenced this pull request Oct 12, 2015
@dmethvin @markelog
Event: Move .bind() and .delegate() to deprecated
7e78c2e 
Cherry-picked from ee0854f
Fixes gh-2288
Closes gh-2624
@dmethvin dmethvin closed this in ee0854f Oct 12, 2015
@dmethvin dmethvin deleted the 2288-deprecate-event branch October 20, 2015 16:47
@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@dmethvin @mgol @timmywil @markelog @jquerybot