Bug Tracker

Modify

Ticket #7783 (closed enhancement: fixed)

Opened 3 years ago

Last modified 18 months ago

Fixing $.proxy to work like (and use) Function.prototype.bind

Reported by: gf3 Owned by: gf3
Priority: low Milestone: 1.6
Component: core Version: 1.4.4
Keywords: Cc:
Blocking: Blocked by:

Description

As it stands currently, $.proxy is in a weird place. It's likely too advanced of a tool for the average user, and crippled for advanced users. It lacks the ability to partially apply arguments and doesn't take advantage of native implementations where available. As well, $.proxy is being used internally to set matching guids (in $.fn.bind and $.fn.toggle), which is an unnecessary performance hit as well as reliance on an indirect side-effect of $.proxy.

My proposal is to:

  • Make $.proxy spec-compatible.
  • Use native implementations where available.
  • Remove the non-standard object/key syntax (breaking).

Change History

comment:1 Changed 3 years ago by gf3

I've committed a fix which takes care of the following:

  • Makes $.proxy spec compatible.
  • Uses native Function#bind where available.
  • Removes the non-standard object/key syntax.
  • Fixes locations where $.proxy is being used incorrectly internally.
  • Added tests to support new functionality.

The bind fallback I used is based on research from the  Prototype bug tracker and is ultimately by Juriy Zaytsev (@kangax).

Native bind is used where possible, which is  over twice as fast as jQuery's current solution in Chrome 9.

The non-standard object/key syntax has been removed. Yes, this is a breaking change, however it is trivial to use the proper syntax. It adds unnecessary complexity for virtually zero gain.

Internally jQuery was using $.proxy only to set matching guides on related functions. This caused unnecessary overhead. And is just plain wrong. I've fixed this (which I would label as an actual bug).

Patch is on github, a pull-request can be made:  https://github.com/gf3/jquery/commit/9f8cd6c499844451468257140e71f611abb3a040

comment:2 Changed 3 years ago by ajpiano

  • Component changed from unfiled to core

I am fully in favour of this landing - partial application is worth it alone, and I agree - the stringy syntax is really not necessary and kinda feels yucky, so I am not averse to losing it. If people have used it in the past year since 1.4 came out, I think we can guide them toward the proper syntax...

What do other people think about this for 1.5?

comment:3 Changed 3 years ago by snover

I’m not sure what the project’s outlook has been with regards to removing things outright like that in a new version without deprecating it for a release.

(Unrelatedly, I also kind of feel like the method signature should be (thisObj, fn, args…) since that matches other functions that rebind this like Function.call and Function.apply, but that probably won’t happen since it is an even more extreme bc break.)

comment:4 Changed 3 years ago by rwaldron

  • Owner set to gf3
  • Status changed from new to assigned

Back when this was in discussion on the jQuery Core google group, I had argued the same points - and ultimately $.proxy was the outcome - so I'm gladly in favor of this change, I also think (thisObj, fn, args…) should be used - however the function def would be nasty overloaded with ugly argument manipulation.

Last edited 3 years ago by rwaldron (previous) (diff)

comment:5 Changed 3 years ago by gf3

Replying to rwaldron:

Back when this was in discussion on the jQuery Core google group, I had argued the same points - and ultimately $.proxy was the outcome - so I'm gladly in favor of this change, I also think (thisObj, fn, args…) should be used - however the function def would be nasty overloaded with ugly argument manipulation.

Replying to snover:

(Unrelatedly, I also kind of feel like the method signature should be (thisObj, fn, args…) since that matches other functions that rebind this like Function.call and Function.apply, but that probably won’t happen since it is an even more extreme bc break.)

I've followed the style of $.map et al, where the instance which would normally be acted upon is the first argument. E.g.:

ar.forEach( fn ) → $.map( ar, fn )

fn.bind( context ) → $.proxy( fn, context )

fn.bind( context, arg1, arg2 ) → $.proxy( fn, context, arg1, arg2 )

I think if Function#call were implemented in jQuery it would have the following signature: $.call( fn, context, arg1, argN).

Last edited 3 years ago by gf3 (previous) (diff)

comment:6 Changed 3 years ago by gf3

Added a commit to add a quick test to $.support for native bind, as per ajpiano's suggestion:

 https://github.com/gf3/jquery/commit/5b1b57850cfd4c92a4f9231581dff7faac489566

comment:7 Changed 3 years ago by gf3

comment:8 Changed 3 years ago by rwaldron

  • Priority changed from undecided to low
  • Milestone 1.next deleted

comment:9 Changed 3 years ago by rwaldron

  • Milestone set to 1.5

comment:10 Changed 3 years ago by gf3

Added the old syntax back in.

comment:11 Changed 3 years ago by john

  • Status changed from assigned to closed
  • Resolution set to fixed
  • Milestone set to 1.6

Landed.

comment:12 Changed 3 years ago by rwaldron

  • Keywords needsdocs added
  • Status changed from closed to reopened
  • Resolution fixed deleted
  • Milestone 1.6 deleted

comment:13 Changed 3 years ago by rwaldron

note that new docs should cover the latest behaviour after the removal of Function.prototype.bind

 https://github.com/jquery/jquery/commit/15da298f72bf94a95563abc12b8e6fec8c604099

comment:14 Changed 3 years ago by timmywil

  • Status changed from reopened to closed
  • Resolution set to fixed
  • Milestone set to 1.6

It's ok to have closed tickets that need docs, just add keyword needsdocs.

comment:15 Changed 3 years ago by timmywil

#9679 is a duplicate of this ticket.

comment:16 Changed 18 months ago by mikesherov

  • Keywords needsdocs removed

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.