Skip to content
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

Ability to avoid expando collisions #29

Conversation

SlexAxton
Copy link
Member

The potential bug I reported is here: http://dev.jquery.com/ticket/6842

I want to be able to guarantee that my expando doesn't collide with another. V8 is now scarily close to being able to instantiate two instances of jQuery in the same millisecond. The only way I can think of to reliably do this (keeping in mind the case where two copies of jQuery are deeply noConflicted), is to explicitly pass in a unique prefix.

This patch reduces normal conflicts between different versions of jQuery by adding in the version to the expando, and also exposes a second parameter to jQuery.noConflict

For example:

myJQ = jQuery.noConflict(true, 'myUniquePrefix');

Generally, this is necessary for third party widgets that use jQuery and don't want to collide with whatever is on the page.

I did add a check to not allow a prefix to be added if the jQuery.cache had been used, that way no connections would be lost between old expando values. The best-practice here being that you call this new noConflict(true, 'prefix') before you use anything in the library.

Test cases are provided (and passing) for both the case when a prefix is added with or without an empty jQuery.cache.

…ix. Also put the version number in the expando. Doesn't allow prefix after jQuery.cache has been used. Fixes #6842.
@codenothing
Copy link

What about just moving expando defn up to core.js, and altering like so:

expando: "jQuery" + ( _jQuery && _jQuery.expando ? _jQuery.expando : jQuery.now() )

That way it's only a single line change and should to never conflict.

@SlexAxton
Copy link
Member Author

That ignores the possibility of the other jQuery being noConflicted as well... unlikely but more common then you'd probably guess.

@rkatic
Copy link
Contributor

rkatic commented Sep 30, 2010

I am wondering, why not to set the expando to the window to "reserve" it..
Immediately after defining expando we can firstly chek if it is already "reserved" and if it is, change it a bit..

while( expando in window ) {
    expando += 'x';
}

// Reserve it!
window[ expando ] = ...

@SlexAxton
Copy link
Member Author

I was avoiding leaking anything else into the global namespace.

@rkatic
Copy link
Contributor

rkatic commented Sep 30, 2010

Ok, but there is still possible that same prefix is used.. an random number would be even more solid for me.

@rkatic
Copy link
Contributor

rkatic commented Sep 30, 2010

Also it is possible to loop while a millisecond is passed...
After expando definition:

var _now = +new Date()
    expando = ... + _now;

while ( +new Date() === _now ) {} 

@SlexAxton
Copy link
Member Author

The developer could at least ensure that it's unique. The wait a ms method is ok, but it seems a little greedy to double the time via a blocking loop that jQuery takes to instantiate.

@rkatic
Copy link
Contributor

rkatic commented Oct 1, 2010

IMO looping one ms is not so greedy as expecting from the user to provide unique prefixes only because internally needed.

Also take in consideration that the expando could be used in plugins before the noConflict call. For example because the data() and event API can be used against native objects too.

@SlexAxton
Copy link
Member Author

That's penalizing all users for the need of 1% of users. If you know you need to ensure expando uniqueness, you should be able to prefix it.

@SlexAxton
Copy link
Member Author

I test against the case that the expando has already been used.

@rkatic
Copy link
Contributor

rkatic commented Oct 1, 2010

Are you saying that only because the cache is used that one ms is certainty passed, even if entire jQuery initialization takes less then one ms?

If not, you should at least raise an error if a prefix is given and cache is not empty.

@rkatic
Copy link
Contributor

rkatic commented Oct 1, 2010

Also we can loop one ms in outro.js so there would not be penalties (entire jQuery init takes normally ~5 ms).

// In core.js
jQuery._now = jQuery.now();

// In data.js
jQuery.expando = ... + jQuery._now;

// In outro.js
while ( jQuery.now() === jQuery._now ) {}

@rkatic
Copy link
Contributor

rkatic commented Oct 1, 2010

Sorry, I realized that I was not clear enough...

I test against the case that the expando has already been used.

As I said, data() and event API can be used against native objects too. And in case of natives, expando is used without using the cache. But even if your test was good enough, silently not applied prefixes (in case of empty cache) is dangerous if jQuery initialization takes less then 1ms.

@jeresig
Copy link
Member

jeresig commented Oct 8, 2010

What if we just changed expando: ... to:

expando: "jQuery" + (Math.random() * 10E15),

That will be damn hard to duplicate (probably akin to winning a few lotteries simultaneously).

@rkatic
Copy link
Contributor

rkatic commented Oct 8, 2010

IMO, that would be enough if it is hard to expect hundreds of jQuery instances in same window. Is it?

@SlexAxton
Copy link
Member Author

It's a bit of a hack, but I don't think I mind... I still think we should add the $.fn.jquery string in there since a lot of times the reason for loading to jQuery's is for two different versions.

@cowboy
Copy link
Member

cowboy commented Oct 9, 2010

As long as jQuery.expando is now used consistently throughout the source (and there are no remaining references to an expando var) I don't see any reason why jQuery.expando can't just be set if necessary, immediately after jQuery loads (and before any data are set).

$.getScript( 'my-jquery.js', function(){
  window.myjQuery = $.noConflict( true );
  myjQuery.expando = 'myUniqueExpando';
});

This keeps the jQuery.noConflict signature simple.

@rkatic
Copy link
Contributor

rkatic commented Oct 10, 2010

@cowboy: jQuery.expando have to be considered read-only for at least two reasons: 1) Plugin could uses data, event and similar APIs before the $.noConflict call; 2) Only some values are valid ones and that is an internal detail (rinlinejQuery).

EDIT: THe first reason is not entirely true, becouse you could set jQuery.expando before plugin scripts, but even then, it would be a pain for users to bother about that.

@rkatic
Copy link
Contributor

rkatic commented Oct 10, 2010

Regarding the random number solution, although in my opinion it is a fine solution, I am a bit worried about the possibility that the random generator is not always good enough as we could expect (precision, poor implementation, eventual OS bug..).

@paulirish
Copy link
Member

"jQuery" + (Math.random() * 10E15) sounds perfectly adequate. Worrying about random numbers not being random enough is for people above our pay grade. :)

@rkatic
Copy link
Contributor

rkatic commented Oct 10, 2010

@paulirish: Heh, you could be surprised what some people do in name of optimization (mobile)... but probably you are right, probably I am too worried about that. :)

@cowboy
Copy link
Member

cowboy commented Oct 10, 2010

@rkatik my point is simply that in Alex's use-case it makes more sense to manipulate the jQuery.expando property directly, instead of complicating the jQuery.noConflict signature by adding an additional argument.

Also, I'd use 9e9 instead of 10e15 because it's still adequately large and saves a whopping 2 bytes in the download :-)

@lock lock bot locked as resolved and limited conversation to collaborators Jan 20, 2019
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants