Skip to main content

Bug Tracker

Side navigation

#13554 closed bug (fixed)

Opened March 02, 2013 11:58PM UTC

Closed April 02, 2013 10:26PM UTC

Move [un]bind & [un]delegate to event-alias

Reported by: anthonyryan1 Owned by:
Priority: low Milestone: 1.10
Component: event Version: git
Keywords: Cc:
Blocked by: Blocking:
Description

Proposal

I would like to make a case that these functions are no longer a part of the core event functionality, but rather belong in event-alias with all of the other aliases that abstract .on() & .off().

All of these methods merely act as wrappers for .on() / .off() providing a different name and argument order, making them very similar to all the other event-alias methods.

Pros

  • Better separation of event handling code in event.js, and various abstractions on top of it in event-alias.js.
  • Minor decrease in filesize when building without event-alias.

Cons

  • Minor increase in filesize for standard build.
Attachments (0)
Change History (14)

Changed March 03, 2013 03:17AM UTC by scottgonzalez comment:1

Cons:

  • Breaks tons of existing code.
  • Making this move now (at a point where jQuery UI still supports version of core that do not have .on()/.off()) means that jQuery UI would have to shim them anyway.

The former is more important in that the savings of leaving out these methods is very low and the amount of code that would break with event-alias removed is very high (much higher than just losing the shortcuts).

Changed March 03, 2013 03:26AM UTC by anthonyryan1 comment:2

This shouldn't break any code at all, since it's simply moving it from one module to another with other methods much like them.

.hover(), .click(), .blur(), .mouseover(), etc. never broke any code in the default build when they moved into event-alias.

Also, isn't the nature of explicitly building jQuery without event-alias to use a very strict (recommended) subset of the event API? As such, there's no additional code expected to break because a person would need to explicitly build jQuery without these included.

Changed March 03, 2013 03:30AM UTC by dmethvin comment:3

We had a decent technical reason for removing .live() but I agree with Scott that there's not a lot of benefit size-wise to removing them. I'd prefer to not deprecate them and thus send a message that they might disappear causing chaos.

It would be nice to replace the scattered usages of .bind() and .delegate() in the unit tests just to keep things clean. If that was done we could move the methods to event-alias.js perhaps?

Changed March 03, 2013 03:41AM UTC by anthonyryan1 comment:4

_comment0: I don't understand why this would be viewed as a deprecation. I'm simply proposing an organizational change to limit event.js to the recommended event API, and using event-alias for shortcuts, legacy methods, and other wrappers around the core event API (which appeared to be it's current purpose). \ \ None of the shortcut methods I already mentioned, .click(), .blur(), etc. have any indication in the API docs that they're deprecated, yet they've been moved into event-alias. \ \ Is there an some intention to deprecate everything in event-alias in the near future that I'm unaware of? \ \ Additionally, I would not be opposed to helping to migrate all of the .bind & .delegate tests to .on & .off, which would be a pre-requisite to ensure all tests can still be run with these methods excluded.1362282165292360

I don't understand why this would be viewed as a deprecation. I'm simply proposing an organizational change to limit event.js to the recommended event API, and using event-alias for shortcuts, legacy methods, and other wrappers around the core event API (which appeared to be its current purpose).

None of the shortcut methods I already mentioned, .click(), .blur(), etc. have any indication in the API docs that they're deprecated, yet they've been moved into event-alias.

Is there an some intention to deprecate everything in event-alias in the near future that I'm unaware of?

Additionally, I would not be opposed to helping to migrate all of the .bind & .delegate tests to .on & .off, which would be a pre-requisite to ensure all tests can still be run with these methods excluded.

Changed March 03, 2013 04:16AM UTC by dmethvin comment:5

Sure, moving them to event-alias sounds good, but like I said we'd need to fix the unit tests to do that. I don't think it's hard but it probably is tedious. If you're interested in doing the work I'd land it though.

Changed March 04, 2013 03:06AM UTC by m_gol comment:6

_comment0: I'm all for cleaning up the codebase so here's my try at this ticket:[[BR]] \ https://github.com/jquery/jquery/pull/11931362366399197861

I'm all for cleaning up the codebase so here's my shot at this ticket:

https://github.com/jquery/jquery/pull/1193

Changed March 06, 2013 09:16PM UTC by m_gol comment:7

@dmethvin, will you look at my pull request? rwaldron said he leaved it for you to sign & land.

Changed March 06, 2013 09:18PM UTC by dmethvin comment:8

component: unfiledevent
milestone: None1.9.2
priority: undecidedlow
status: newopen

Don't worry if it takes a little while, I usually sit down and do a bunch of stuff at once.

Changed March 06, 2013 10:00PM UTC by m_gol comment:9

Sure! I'll wait.

Changed March 26, 2013 02:16AM UTC by Michał Gołębiowski comment:10

bind/unbind changed to on/off in unit tests; refs #13554

Changeset: 8ca9f931ec311b6f73990eac7665451a28bceac3

Changed March 26, 2013 02:16AM UTC by Michał Gołębiowski comment:11

delegate/undelegate changed to on/off in unit tests; refs #13554

Changeset: 84a94acae1ed7d65d91df235985e433d34486dc3

Changed March 26, 2013 02:16AM UTC by Michał Gołębiowski comment:12

moved bind, unbind, delegate & undelegate to event-alias.js; refs #13554

Changeset: 100d3c351604e1f9641098da2e78678b4e6d9cdf

Changed March 26, 2013 06:18PM UTC by dmethvin comment:13

milestone: 1.9.21.10

Changed April 02, 2013 10:26PM UTC by m_gol comment:14

resolution: → fixed
status: openclosed