Skip to content

Commit

Permalink
fix(globals): Use shallow copy to update the globals.params / $state.…
Browse files Browse the repository at this point in the history
…params object

Previously, the `globals.params` object was updated using a deep copy.
This causes problems when object-type parameter values aren't cloneable.

Now, the parameter values are copied to the `globals.params` object using a shallow copy, i.e., `Object.assign`.
This is consistent with parameter handling in the `PathNode` code also.

If this commit is causing problems, it is likely that app code is mutating
the existing object parameters and then calling `state.go` with the mutated
parameters.  Recommend making a clone of the params before calling `state.go`.

Closes #74
  • Loading branch information
christopherthielen committed Sep 21, 2017
1 parent 2f1ae9a commit e883afc
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 8 deletions.
6 changes: 1 addition & 5 deletions src/common/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ let w: any = typeof window === 'undefined' ? {} : window;
let angular = w.angular || {};
export const fromJson = angular.fromJson || JSON.parse.bind(JSON);
export const toJson = angular.toJson || JSON.stringify.bind(JSON);
export const copy = angular.copy || _copy;
export const forEach = angular.forEach || _forEach;
export const extend = Object.assign || _extend;
export const equals = angular.equals || _equals;
Expand Down Expand Up @@ -528,11 +527,8 @@ export function tail<T>(arr: T[]): T {

/**
* shallow copy from src to dest
*
* note: This is a shallow copy, while angular.copy is a deep copy.
* ui-router uses `copy` only to make copies of state parameters.
*/
function _copy(src: Obj, dest: Obj) {
export function copy(src: Obj, dest?: Obj) {
if (dest) Object.keys(dest).forEach(key => delete dest[key]);
if (!dest) dest = {};
return extend(dest, src);
Expand Down
2 changes: 0 additions & 2 deletions src/globals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ import {StateDeclaration} from "./state/interface";
import {StateObject} from "./state/stateObject";
import {Transition} from "./transition/transition";
import {Queue} from "./common/queue";
import {TransitionService} from "./transition/transitionService";
import {copy} from "./common/common";
import { Disposable } from './interface';

/**
Expand Down
23 changes: 22 additions & 1 deletion test/stateServiceSpec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,27 @@ describe('stateService', function () {

done();
});

it('performs a shallow copy to $stateParams (for object parameters)', async (done) => {
const x = { foo: '123' }, y = { bar: 'abc' };
await $state.go('D', { x, y });

expect($state.params.x).toBe(x);
expect($state.params.y).toBe(y);

done();
});

it('updates the object reference in-place', async (done) => {
const params = $state.params;
const x = { foo: '123' };
await $state.go('D', { x });

expect($state.params.x).toBe(x);
expect(params).toBe($state.params);

done();
});
});
});

Expand Down Expand Up @@ -952,4 +973,4 @@ describe('stateService', function () {
});

});
});
});

0 comments on commit e883afc

Please sign in to comment.