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

#7783 - Fixing $.proxy to work like (and use) Function.prototype.bind #133

Merged
merged 7 commits into from Apr 10, 2011

Conversation

Projects
None yet
9 participants
@gf3
Contributor

gf3 commented Dec 16, 2010

@ajpiano

This comment has been minimized.

Show comment
Hide comment
@ajpiano

ajpiano Dec 15, 2010

We should probably drop a test for this into jQuery.support, testing for existence on every $.proxy call seems a waste of time....

We should probably drop a test for this into jQuery.support, testing for existence on every $.proxy call seems a waste of time....

This comment has been minimized.

Show comment
Hide comment
@gf3

gf3 Dec 15, 2010

Owner

Added in: 5b1b578, good call.

Owner

gf3 replied Dec 15, 2010

Added in: 5b1b578, good call.

@csnover

This comment has been minimized.

Show comment
Hide comment
@csnover

csnover Dec 16, 2010

Could you mention which browsers this currently fails in? That way, we know later on when we’re dropping old browsers which areas of code we can get rid of by removing the alternate code paths.

Could you mention which browsers this currently fails in? That way, we know later on when we’re dropping old browsers which areas of code we can get rid of by removing the alternate code paths.

This comment has been minimized.

Show comment
Hide comment
@gf3

gf3 Dec 16, 2010

Owner

Added in: 1ebb5ab.

Owner

gf3 replied Dec 16, 2010

Added in: 1ebb5ab.

@jaubourg

This comment has been minimized.

Show comment
Hide comment
@jaubourg

jaubourg Dec 16, 2010

Member

Hehe, seems to me you misunderstood snover's request. ;)

The goal is to know which browsers do not support the feature so that when we drop support for all of them we'll be able to remove the property and all tests depending on it.

Nice patch nonetheless :)

Member

jaubourg commented on 1ebb5ab Dec 16, 2010

Hehe, seems to me you misunderstood snover's request. ;)

The goal is to know which browsers do not support the feature so that when we drop support for all of them we'll be able to remove the property and all tests depending on it.

Nice patch nonetheless :)

This comment has been minimized.

Show comment
Hide comment
@gf3

gf3 Dec 16, 2010

Contributor

Heh, I just thought it might be easier to list which ones currently support Function#bind. Should I reverse this?

Contributor

gf3 replied Dec 16, 2010

Heh, I just thought it might be easier to list which ones currently support Function#bind. Should I reverse this?

This comment has been minimized.

Show comment
Hide comment
@jaubourg

jaubourg Dec 17, 2010

Member

Well, in the goal of knowing when to remove the property, a list of browsers not supporting is better, yes.

Member

jaubourg replied Dec 17, 2010

Well, in the goal of knowing when to remove the property, a list of browsers not supporting is better, yes.

This comment has been minimized.

Show comment
Hide comment
@gf3

gf3 Dec 19, 2010

Contributor

Updated: ade531c.

Contributor

gf3 replied Dec 19, 2010

Updated: ade531c.

@fearphage

This comment has been minimized.

Show comment
Hide comment
@fearphage

fearphage Dec 18, 2010

Why not fork here like the simulated bind based on args.length? call is still faster than apply.

Why not fork here like the simulated bind based on args.length? call is still faster than apply.

This comment has been minimized.

Show comment
Hide comment
@gf3

gf3 Dec 19, 2010

Owner

Good call. BAM: 6bc9fc7.

Owner

gf3 replied Dec 19, 2010

Good call. BAM: 6bc9fc7.

@mathiasbynens

This comment has been minimized.

Show comment
Hide comment
@mathiasbynens

mathiasbynens Dec 31, 2010

By >= you mean <=, right?

By >= you mean <=, right?

This comment has been minimized.

Show comment
Hide comment
@gf3

gf3 Jan 2, 2011

Owner

Yes, whoops.

Owner

gf3 replied Jan 2, 2011

Yes, whoops.

@cowboy

This comment has been minimized.

Show comment
Hide comment
@cowboy

cowboy Jan 3, 2011

Member

Can an approach like this be taken so that the new method is backwards compatible (with both documented signatures)?

https://gist.github.com/a9634a9785e274d64e39

Member

cowboy commented Jan 3, 2011

Can an approach like this be taken so that the new method is backwards compatible (with both documented signatures)?

https://gist.github.com/a9634a9785e274d64e39

@cowboy

This comment has been minimized.

Show comment
Hide comment
@cowboy

cowboy Jan 3, 2011

Member

Ok, here's that approach, a bit more fleshed out (yet totally untested).

https://gist.github.com/a9634a9785e274d64e39

Member

cowboy commented Jan 3, 2011

Ok, here's that approach, a bit more fleshed out (yet totally untested).

https://gist.github.com/a9634a9785e274d64e39

@wycats

This comment has been minimized.

Show comment
Hide comment
@wycats

wycats Jan 3, 2011

Member

+1 for backwards compatibility

Member

wycats commented Jan 3, 2011

+1 for backwards compatibility

gf3 added some commits Jan 21, 2011

Merge branch 'master' into proxy-native-bind
* master: (194 commits)
  Revert "Make sure that focusin/focusout bubbles in non-IE browsers." This was causing problems with the focusin event, see: #7340.
  Replaces "text in-between" technique with a full-fledged one-level transitive search for converters (unit tests added). Also cleans up auto dataType determination and adds converter checks in order to guess the best dataType possible.
  Moves determineResponse logic into main ajax callback. Puts responseXXX fields definitions into ajaxSettings.
  Removes misleading comment.
  Bring jQuery('#id') and jQuery('body') logic back into core (while leaving it in Sizzle at the same time). Was causing too much of a performance hit to leave it all to Sizzle.
  Renames Deferred's fire and fireReject methods as resolveWith and rejectWith respectively.
  Fix typo in regex tweak from previous commit.
  Renames determineDataType as determineResponse. Makes it more generic as a first step into integrating the logic into the main ajax done callback. Also fixes some comments in ajax/xhr.js.
  Move jQuery(...) selector speed-up logic into Sizzle(...) qSA handling. Additionally add in a new catch for Sizzle('.class') (avoid using qSA and use getElementsByClassName instead, where applicable).
  Revises the way arguments are handled in ajax.
  Makes sure statusCode callbacks are ordered in the same way success and error callbacks are. Unit tests added.
  Cleans up and simplifies code shared by ajaxPrefilter and ajaxTransport. Removes chainability of ajaxSetup, ajaxPrefilter and ajaxTransport. Also makes sure context is handled properly by ajaxSetup (unit test added).
  Rework unit tests to check actual result elements.
  Moves active counter test after all other ajax tests where it should be.
  Revised the Nokia support fallback. It turns out that Nokia supports the documentElement property but does not define document.compatMode. Adding this third fallback allows Nokia to run jQuery error-free and return proper values for window width and height.
  Moves things around to make jsLint happier.
  Fixes crossDomain test so that it assumes port to be 80 for http and 443 for https when it is not provided.
  Moves determineDataType into ajaxSettings so that it is accessible to transports without the need for a second argument and so that we can now pass the original options to the transport instead. Also ensures the original options are actually propagated to prefilters (they were not).
  Re-adds hastily removed variable and simplifies statusCode based callbacks handling.
  Use undefined instead of 0 to deference transport for clarity.
  ...

Conflicts:
	src/event.js
@gf3

This comment has been minimized.

Show comment
Hide comment
@gf3

gf3 Jan 21, 2011

Contributor

Added old syntax back in.

Contributor

gf3 commented Jan 21, 2011

Added old syntax back in.

@jeresig jeresig merged commit 574ae3b into jquery:master Apr 10, 2011

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