Skip to content

Expando Collision Handling #50

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

Closed
wants to merge 3 commits into from
Closed

Conversation

SlexAxton
Copy link
Member

Updated this pull request with the feedback from everyone.

…, so collisions become less likely. Also added jQuery version to instantly differentiate separate versions of jQuery (a common use case for noConflict, etc, when two jQuery instances are on the page)
@rkatic
Copy link
Contributor

rkatic commented Oct 14, 2010

Sorry, for late answer.
This looks good. My last suggestion would be to change the expression to /\D/g (bringing "jQuery" outside of parenthesis), so the expando would be valid even with pre versions of jQuery (like 1.4.3pre).

PS: I don't think it is bad to leave more commits if you are adding changes. Reclosing/reopening the same pull req. splits the same discussion.

@SlexAxton
Copy link
Member Author

How do I do a pull request of a new patch without closing one and opening another?

@rkatic
Copy link
Contributor

rkatic commented Oct 14, 2010

If you made pull req. from your master (and you did), then successively pushed commits to that branch will be auto-added to the req. Hope I was clear.

@SlexAxton
Copy link
Member Author

Awesome, I was unaware of that feature.

@rkatic
Copy link
Contributor

rkatic commented Oct 14, 2010

Indeed! This pull request system is a bless. :)

EDIT: Also, when you begin to open more pull req., then you can for each create a separate branch, so commits will not be mixed, but req. will still be updated..

@SlexAxton
Copy link
Member Author

As for the change that you suggest, is there anything invalid about jQuery143pre123123123123 ? We wouldn't want the 'pre' version and the release version to evaluate to the same expando anyway...

@rkatic
Copy link
Contributor

rkatic commented Oct 14, 2010

With current rinlinejQuery, that would not be a valid expando. Don't think "pre" is playing to much here to avoid collisions. We heve to support multiple jQuery-es of same version anyway (to prevent cases of plugin collisions..).

@SlexAxton
Copy link
Member Author

I went ahead and updated it. Thanks for the fix.

@csnover
Copy link
Member

csnover commented Nov 21, 2010

This is for ticket #6842.

@csnover
Copy link
Member

csnover commented Dec 27, 2010

Committed.

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

3 participants