Bug Tracker

Modify

Ticket #13554 (closed bug: fixed)

Opened 14 months ago

Last modified 13 months ago

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

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

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.

Change History

comment:1 Changed 14 months ago by scott.gonzalez

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

comment:2 Changed 14 months ago by anthonyryan1

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.

comment:3 Changed 14 months ago by dmethvin

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?

comment:4 Changed 14 months ago by anthonyryan1

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.

Last edited 14 months ago by anthonyryan1 (previous) (diff)

comment:5 Changed 14 months ago by dmethvin

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.

comment:6 Changed 14 months ago by m_gol

I'm all for cleaning up the codebase so here's my shot at this ticket:
 https://github.com/jquery/jquery/pull/1193

Last edited 14 months ago by m_gol (previous) (diff)

comment:7 Changed 14 months ago by m_gol

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

comment:8 Changed 14 months ago by dmethvin

  • Priority changed from undecided to low
  • Status changed from new to open
  • Component changed from unfiled to event
  • Milestone changed from None to 1.9.2

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

comment:9 Changed 14 months ago by m_gol

Sure! I'll wait.

comment:10 Changed 13 months ago by Michał Gołębiowski

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

Changeset: 8ca9f931ec311b6f73990eac7665451a28bceac3

comment:11 Changed 13 months ago by Michał Gołębiowski

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

Changeset: 84a94acae1ed7d65d91df235985e433d34486dc3

comment:12 Changed 13 months ago by Michał Gołębiowski

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

Changeset: 100d3c351604e1f9641098da2e78678b4e6d9cdf

comment:13 Changed 13 months ago by dmethvin

  • Milestone changed from 1.9.2 to 1.10

comment:14 Changed 13 months ago by m_gol

  • Status changed from open to closed
  • Resolution set to fixed

Please follow the  bug reporting guidlines and use  jsFiddle when providing test cases and demonstrations instead of pasting the code in the ticket.

View

Add a comment

Modify Ticket

Action
as closed
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.