Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Add .noConflict() functionality #8596

Open
wants to merge 2 commits into
base: v1.2.x
Choose a base branch
from
Open

Add .noConflict() functionality #8596

wants to merge 2 commits into from

Conversation

hekike
Copy link

@hekike hekike commented Aug 13, 2014

  • add .noConflict() to the public api

@gergelyke
Copy link

Up! Would love to see this feature landing in an upcoming release!

@caitp
Copy link
Contributor

caitp commented Aug 13, 2014

We had this before, but it was removed last year (9faabd1)

@matsko what was the reasoning for this, btw?

@hekike
Copy link
Author

hekike commented Aug 13, 2014

@caitp yes but I've solved the "resolve global JSONP conflicts" issue with a random callback hash instead of counter.
And I also create a fresh scope in every cases angular = window.angular = {}

@hekike
Copy link
Author

hekike commented Aug 13, 2014

@caitp you can find the reason here: #2893

@caitp
Copy link
Contributor

caitp commented Aug 13, 2014

I'm pretty sure this is going to break tests due to making callback IDs unpredictable (there are tests in httpBackendSpec.js which depend on this)

You'd need to mock Math.random() to make those work

@hekike
Copy link
Author

hekike commented Aug 13, 2014

@caitp do you mean for this linecallbacks = {counter: 0};?
Because it's just reset the counter, which I don't use :)

Running "tests:jqlite" (tests) task
INFO [karma]: Karma v0.12.9 server started at http://localhost:9876/
INFO [launcher]: Starting browser Chrome
INFO [Chrome 36.0.1985 (Mac OS X 10.9.4)]: Connected on socket aGnc95_afDkshaIXy3ib with id 96156252
Chrome 36.0.1985 (Mac OS X 10.9.4): Executed 1241 of 3059 SUCCESS (0 secs / 6.017 secs)
Chrome 36.0.1985 (Mac OS X 10.9.4): Executed 3059 of 3059 SUCCESS (12.411 secs / 12.782 secs)

Running "tests:jquery" (tests) task
INFO [karma]: Karma v0.12.9 server started at http://localhost:9876/
INFO [launcher]: Starting browser Chrome
INFO [Chrome 36.0.1985 (Mac OS X 10.9.4)]: Connected on socket 5q0Jst9Mm8q4CDhBy8a1 with id 9428067
Chrome 36.0.1985 (Mac OS X 10.9.4): Executed 1250 of 3068 SUCCESS (0 secs / 6.127 secs)
Chrome 36.0.1985 (Mac OS X 10.9.4): Executed 3068 of 3068 SUCCESS (13.013 secs / 13.359 secs)

Running "tests:modules" (tests) task
INFO [karma]: Karma v0.12.9 server started at http://localhost:9876/
INFO [launcher]: Starting browser Chrome
INFO [Chrome 36.0.1985 (Mac OS X 10.9.4)]: Connected on socket mEnyMG5XuTYT5i4XzAzr with id 37546992
Chrome 36.0.1985 (Mac OS X 10.9.4): Executed 429 of 429 SUCCESS (1.743 secs / 1.687 secs)

Done, without errors.

@caitp
Copy link
Contributor

caitp commented Aug 13, 2014

No, I mean because you're generating a random key and adding it to the object rather than adding a predictable incremented number.

@gergelyke
Copy link

But why do angular need an incremented number?

@caitp
Copy link
Contributor

caitp commented Aug 13, 2014

Nah nevermind it should be fine, the tests use the replaced string to get the callbacks property name, so it doesn't matter.

@mary-poppins
Copy link

I'm sorry, but I wasn't able to verify your Contributor License Agreement (CLA) signature. CLA signature is required for any code contributions to AngularJS.

Please sign our CLA and ensure that the CLA signature email address and the email address in this PR's commits match.

If you signed the CLA as a corporation, please let us know the company's name.

Thanks a bunch!

PS: If you signed the CLA in the past then most likely the email addresses don't match. Please sign the CLA again or update the email address in the commit of this PR.
PS2: If you are a Googler, please sign the CLA as well to simplify the CLA verification process.

@hekike
Copy link
Author

hekike commented Aug 14, 2014

@mary-poppins sry my author email address was wrong. Now it's okay.

@mary-poppins
Copy link

CLA signature verified! Thank you!

Someone from the team will now triage your PR and it will be processed based on the determined priority (doc updates and fixes with tests are prioritized over other changes).

@brandonburkett
Copy link

Super interested in this feature. Hope it makes it in soon.

@AoDev
Copy link

AoDev commented Nov 19, 2014

Hello, I have been checking on this issue for a while now. Three months have passed since this PR, any chance to see this feature soon? it's a blocker for a lot of people who want to make third-party apps with Angular.

@americos
Copy link

+1 We would love to have this feature

@@ -46,7 +46,7 @@ function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDoc
url = url || $browser.url();

if (lowercase(method) == 'jsonp') {
var callbackId = '_' + (callbacks.counter++).toString(36);
var callbackId = '_' + Math.random().toString(36).substring(7);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the "birthday paradox", if you have 100,000 callbacks, you will have a 6% chance of a collision. (See https://instacalc.com/28845, with "number of days" = 78364164096 = 36^7, and "number of people" = 100000.) So, I think this needs to be a longer random number.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we only care for the callback id while the request is in flight, we care about the number of concurrent JSONP requests. If you had 100000 of them, then 6% callback id collision rate would be the least of your problems 😛

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.