Skip to content
Permalink
Browse files

fix(common): update $locationShim to notify onChange listeners before…

… emitting AngularJS events (#32037)

The $locationShim has onChange listeners to allow for synchronization logic between
AngularJS and Angular. When the AngularJS routing events are emitted first, this can
cause Angular code to be out of sync. Notifying the listeners earlier solves the
problem.

PR Close #32037
  • Loading branch information...
agale123 authored and kara committed Aug 7, 2019
1 parent bef27f2 commit 5064dc75acbd86c0afe23d6ef46117ca65470b47
Showing with 41 additions and 11 deletions.
  1. +4 −1 packages/common/upgrade/src/location_shim.ts
  2. +37 −10 packages/common/upgrade/test/upgrade.spec.ts
@@ -144,6 +144,7 @@ export class $locationShim {
this.$$parse(oldUrl);
this.state(oldState);
this.setBrowserUrlWithFallback(oldUrl, false, oldState);
this.$$notifyChangeListeners(this.url(), this.$$state, oldUrl, oldState);
} else {
this.initalizing = false;
$rootScope.$broadcast('$locationChangeSuccess', newUrl, oldUrl, newState, oldState);
@@ -199,6 +200,9 @@ export class $locationShim {
}
$rootScope.$broadcast(
'$locationChangeSuccess', newUrl, oldUrl, this.$$state, oldState);
if (urlOrStateChanged) {
this.$$notifyChangeListeners(this.url(), this.$$state, oldUrl, oldState);
}
}
});
}
@@ -415,7 +419,6 @@ export class $locationShim {
// state object; this makes possible quick checking if the state changed in the digest
// loop. Checking deep equality would be too expensive.
this.$$state = this.browserState();
this.$$notifyChangeListeners(url, state, oldUrl, oldState);
} catch (e) {
// Restore old values if pushState fails
this.url(oldUrl);
@@ -628,6 +628,7 @@ describe('$location.onChange()', () => {

let $location: $locationShim;
let upgradeModule: UpgradeModule;
let mock$rootScope: $rootScopeMock;

beforeEach(() => {
TestBed.configureTestingModule({
@@ -640,6 +641,7 @@ describe('$location.onChange()', () => {

upgradeModule = TestBed.get(UpgradeModule);
upgradeModule.$injector = {get: injectorFactory()};
mock$rootScope = upgradeModule.$injector.get('$rootScope');
});

beforeEach(inject([$locationShim], (loc: $locationShim) => { $location = loc; }));
@@ -674,17 +676,44 @@ describe('$location.onChange()', () => {

$location.onChange(changeListener);

// Mock out setting browserUrl
($location as any).browserUrl = (url: string, replace: boolean, state: unknown) => {};

const newState = {foo: 'bar'};
($location as any).setBrowserUrlWithFallback('/newUrl', false, newState);
$location.state(newState);
$location.path('/newUrl');
mock$rootScope.runWatchers();

expect(onChangeVals.url).toBe('/newUrl');
expect(onChangeVals.state).toBe(newState);
expect(onChangeVals.oldUrl).toBe('/');
expect(onChangeVals.state).toEqual(newState);
expect(onChangeVals.oldUrl).toBe('http://host.com');
expect(onChangeVals.oldState).toBe(null);
});

it('should call changeListeners after $locationChangeSuccess', () => {

let changeListenerCalled = false;
let locationChangeSuccessEmitted = false;

function changeListener(url: string, state: unknown, oldUrl: string, oldState: unknown) {
changeListenerCalled = true;
}

$location.onChange(changeListener);

mock$rootScope.$on('$locationChangeSuccess', () => {
// Ensure that the changeListener hasn't been called yet
expect(changeListenerCalled).toBe(false);
locationChangeSuccessEmitted = true;
});

// Update state and run watchers
const stateValue = {foo: 'bar'};
$location.state(stateValue);
mock$rootScope.runWatchers();

// Ensure that change listeners are called and location events are emitted
expect(changeListenerCalled).toBe(true);
expect(locationChangeSuccessEmitted).toBe(true);
});

it('should call forward errors to error handler', () => {

let error !: Error;
@@ -696,10 +725,8 @@ describe('$location.onChange()', () => {

$location.onChange(changeListener, errorHandler);

// Mock out setting browserUrl
($location as any).browserUrl = (url: string, replace: boolean, state: unknown) => {};

($location as any).setBrowserUrlWithFallback('/newUrl');
$location.url('/newUrl');
mock$rootScope.runWatchers();
expect(error.message).toBe('Handle error');
});

0 comments on commit 5064dc7

Please sign in to comment.
You can’t perform that action at this time.