Skip to content

Commit

Permalink
feat($state): allow parameters to pass unharmed
Browse files Browse the repository at this point in the history 
[BREAK] This is a breaking change: state parameters are no longer automatically coerced to strings, and unspecified parameter values are now set to undefined rather than null.
  • Loading branch information
@nateabele
nateabele committed Apr 11, 2014
1 parent b7f074f commit 8939d05
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 39 deletions.
17 changes: 0 additions & 17 deletions src/common.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,23 +108,6 @@ function inheritParams(currentParams, newParams, $current, $to) {
return extend({}, inherited, newParams);
}

/**
* Normalizes a set of values to string or `null`, filtering them by a list of keys.
*
* @param {Array} keys The list of keys to normalize/return.
* @param {Object} values An object hash of values to normalize.
* @return {Object} Returns an object hash of normalized string values.
*/
function normalize(keys, values) {
var normalized = {};

forEach(keys, function (name) {
var value = values[name];
normalized[name] = (value != null) ? String(value) : null;
});
return normalized;
}

/**
* Performs a non-strict comparison of the subset of two objects, defined by a list of keys.
*
Expand Down
6 changes: 3 additions & 3 deletions src/state.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -814,8 +814,8 @@ function $StateProvider( $urlRouterProvider, $urlMatcherFactory) {
return $q.when($state.current);
}

// Normalize/filter parameters before we pass them to event handlers etc.
toParams = normalize(to.params, toParams || {});
// Filter parameters before we pass them to event handlers etc.
toParams = filterByKeys(to.params, toParams || {});

// Broadcast start event and cancel the transition if requested
if (options.notify) {
Expand Down Expand Up @@ -1102,7 +1102,7 @@ function $StateProvider( $urlRouterProvider, $urlMatcherFactory) {
if (!nav || !nav.url) {
return null;
}
return $urlRouter.href(nav.url, normalize(state.params, params || {}), { absolute: options.absolute });
return $urlRouter.href(nav.url, filterByKeys(state.params, params || {}), { absolute: options.absolute });
};

/**
Expand Down
18 changes: 9 additions & 9 deletions test/stateDirectivesSpec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ describe('uiStateRef', function() {
$q.flush();

expect($state.current.name).toEqual('contacts.item.detail');
expect($stateParams).toEqual({ id: "5" });
expect($stateParams).toEqual({ id: 5 });
}));

it('should transition when given a click that contains no data (fake-click)', inject(function($state, $stateParams, $document, $q) {
Expand All @@ -142,7 +142,7 @@ describe('uiStateRef', function() {
$q.flush();

expect($state.current.name).toEqual('contacts.item.detail');
expect($stateParams).toEqual({ id: "5" });
expect($stateParams).toEqual({ id: 5 });
}));

it('should not transition states when ctrl-clicked', inject(function($state, $stateParams, $document, $q) {
Expand All @@ -153,7 +153,7 @@ describe('uiStateRef', function() {
$q.flush();

expect($state.current.name).toEqual('');
expect($stateParams).toEqual({ id: "5" });
expect($stateParams).toEqual({ id: 5 });
}));

it('should not transition states when meta-clicked', inject(function($state, $stateParams, $document, $q) {
Expand All @@ -164,7 +164,7 @@ describe('uiStateRef', function() {
$q.flush();

expect($state.current.name).toEqual('');
expect($stateParams).toEqual({ id: "5" });
expect($stateParams).toEqual({ id: 5 });
}));

it('should not transition states when shift-clicked', inject(function($state, $stateParams, $document, $q) {
Expand All @@ -175,7 +175,7 @@ describe('uiStateRef', function() {
$q.flush();

expect($state.current.name).toEqual('');
expect($stateParams).toEqual({ id: "5" });
expect($stateParams).toEqual({ id: 5 });
}));

it('should not transition states when middle-clicked', inject(function($state, $stateParams, $document, $q) {
Expand All @@ -186,7 +186,7 @@ describe('uiStateRef', function() {
$q.flush();

expect($state.current.name).toEqual('');
expect($stateParams).toEqual({ id: "5" });
expect($stateParams).toEqual({ id: 5 });
}));

it('should not transition states when element has target specified', inject(function($state, $stateParams, $document, $q) {
Expand All @@ -198,7 +198,7 @@ describe('uiStateRef', function() {
$q.flush();

expect($state.current.name).toEqual('');
expect($stateParams).toEqual({ id: "5" });
expect($stateParams).toEqual({ id: 5 });
}));

it('should not transition states if preventDefault() is called in click handler', inject(function($state, $stateParams, $document, $q) {
Expand All @@ -212,7 +212,7 @@ describe('uiStateRef', function() {
$q.flush();

expect($state.current.name).toEqual('');
expect($stateParams).toEqual({ id: "5" });
expect($stateParams).toEqual({ id: 5 });
}));

it('should allow passing params to current state', inject(function($compile, $rootScope, $state) {
Expand Down Expand Up @@ -277,7 +277,7 @@ describe('uiStateRef', function() {
$q.flush();

expect($state.$current.name).toBe("contacts.item.detail");
expect($state.params).toEqual({ id: '5' });
expect($state.params).toEqual({ id: 5 });
}));

it('should resolve states from parent uiView', inject(function ($state, $stateParams, $q, $timeout) {
Expand Down
20 changes: 10 additions & 10 deletions test/stateSpec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ describe('state', function () {
$q.flush();
expect(called).toBeTruthy();
expect($state.current.name).toEqual('DDD');
expect($state.params).toEqual({ x: '1', y: '2', z: '3', w: '4' });
expect($state.params).toEqual({ x: 1, y: 2, z: 3, w: 4 });
}));

it('can defer a state transition in $stateNotFound', inject(function ($state, $q, $rootScope) {
Expand All @@ -282,7 +282,7 @@ describe('state', function () {
$q.flush();
expect(called).toBeTruthy();
expect($state.current.name).toEqual('AA');
expect($state.params).toEqual({ a: '1' });
expect($state.params).toEqual({ a: 1 });
}));

it('can defer and supersede a state transition in $stateNotFound', inject(function ($state, $q, $rootScope) {
Expand Down Expand Up @@ -475,7 +475,7 @@ describe('state', function () {
$q.flush();

expect($state.$current.name).toBe('about.person.item');
expect($stateParams).toEqual({ person: 'bob', id: '5' });
expect($stateParams).toEqual({ person: 'bob', id: 5 });

$state.go('^.^.sidebar');
$q.flush();
Expand Down Expand Up @@ -603,7 +603,7 @@ describe('state', function () {

it('contains the parameter values for the current state', inject(function ($state, $q) {
initStateTo(D, { x: 'x value', z: 'invalid value' });
expect($state.params).toEqual({ x: 'x value', y: null });
expect($state.params).toEqual({ x: 'x value', y: undefined });
}));
});

Expand Down Expand Up @@ -878,24 +878,24 @@ describe('state', function () {

describe('substate and stateParams inheritance', function() {
it('should inherit the parent param', inject(function ($state, $stateParams, $q) {
initStateTo($state.get('root'), {param1: 1});
$state.go('root.sub1', {param2: 2});
initStateTo($state.get('root'), { param1: 1 });
$state.go('root.sub1', { param2: 2 });
$q.flush();
expect($state.current.name).toEqual('root.sub1');
expect($stateParams).toEqual({param1: '1', param2: '2'});
expect($stateParams).toEqual({ param1: 1, param2: 2 });
}));

it('should not inherit siblings\' states', inject(function ($state, $stateParams, $q) {
initStateTo($state.get('root'), {param1: 1});
$state.go('root.sub1', {param2: 2});
initStateTo($state.get('root'), { param1: 1 });
$state.go('root.sub1', { param2: 2 });
$q.flush();
expect($state.current.name).toEqual('root.sub1');

$state.go('root.sub2');
$q.flush();
expect($state.current.name).toEqual('root.sub2');

expect($stateParams).toEqual({param1: '1', param2: null});
expect($stateParams).toEqual({ param1: 1, param2: undefined });
}));
});

Expand Down

2 comments on commit 8939d05

@christopherthielen
Copy link
Contributor

Choose a reason for hiding this comment

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

grunt changelog didn't pick up the [BREAK] tag. I'll manually add this to the changelog.

@nateabele
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, thanks.

Please sign in to comment.