Skip to content
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

Problem with default value of null for a param #1563

Closed
gfreeau opened this issue Nov 18, 2014 · 11 comments
Closed

Problem with default value of null for a param #1563

gfreeau opened this issue Nov 18, 2014 · 11 comments
Assignees
Milestone

Comments

@gfreeau
Copy link

gfreeau commented Nov 18, 2014

I'm seeing an error generated in urlMatcherFactory.js by the code that sets a default value.

This occurs when you have an optional param and a default value of null.

function $value(value) {
  function hasReplaceVal(val) { return function(obj) { return obj.from === val; }; }
  function $replace(value) {
    var replacement = map(filter(self.replace, hasReplaceVal(value)), function(obj) { return obj.to; });
    return replacement.length ? replacement[0] : value;
  }
  value = $replace(value);
  return isDefined(value) ? self.type.decode(value) : $$getDefaultValue();
}

The problem occurs in the hasReplaceVal closure, when obj is null/undefined, obj.from causes the error. 'value' is checked if it is defined until after the call to $replace.

I was using this state config:

url: '/site/{siteId:int}?{startDate:date}&{endDate:date}',
params: {
    endDate: {
        value: null
    }
},

I also tried using the string type instead of the date type and the same result occurred.

Changing the default value to an empty string instead of null, allowed it to work, as obj.to would then return undefined instead of causing an error.

@nitinsurya
Copy link

I also faced a similar problem. 'null' value for state-params works fine till v0.2.11 but gives an error in v0.2.12.

@iblazevic
Copy link

I had that same problem. Handling was changed in 0.2.11 from null to undefined but it worked fine until 0.2.12 version. Its no longer default to null but undefined as noted in CHANGELOG

BREAKING CHANGE: state parameters are no longer automatically coerced to strings, and unspecified parameter values are now set to undefined rather than null.

so undefined should work, hope it helps, I had a problem with it in ui-sref and removing those null parameters work fine as it should from 0.2.11 release

@christopherthielen christopherthielen self-assigned this Nov 20, 2014
@christopherthielen christopherthielen added this to the 0.2.13 milestone Nov 20, 2014
@oliamb
Copy link

oliamb commented Nov 20, 2014

One negative effect of this issue with ui-sref/ui-sref-active:

// for this state
.state('things', {
  url: '/thing/?filterBy',
  templateUrl: 'views/things.html',
  controller: 'thingsCtrl'
})

Up to 0.2.12, this code would work as expected. Either All or Red is active.

<!-- this html raise javascript errors in 0.2.12 -->
<a href ui-sref="things({filterBy: null})" ui-sref-active="active">All</a>
<a href ui-sref="things({filterBy: 'red'})" ui-sref-active="active">Red</a>

Since version 0.2.12, you have to write the code in one of the following form, which all behave in a wrong way (either all is always active or never active).

omitting the key

<!-- this html will not behave as expected -->
<a href ui-sref="things()">All</a>
<a href ui-sref="things({filterBy: 'red'})">Red</a>

using undefined as the key value

<!-- this html will not behave as expected -->
<a href ui-sref="things({filterBy: undefined})">All</a> <!-- yes this is equivalent to the previous -->
<a href ui-sref="things({filterBy: 'red'})">Red</a>

or using an empty string

<!-- this html will not behave as expected -->
<a href ui-sref="things({filterBy: ''})">All</a> <!-- In this case 'all' is never active -->
<a href ui-sref="things({filterBy: 'red'})">Red</a>

@christopherthielen
Copy link
Contributor

I'd like to make sure this is addressed before I release 0.2.13 (which should be real soon now). Can you put this into a plunk for me?

And/or build from master and check it works properly already.

@oliamb
Copy link

oliamb commented Nov 20, 2014

http://plnkr.co/edit/awhez1GjpDrt11uQqeif

I discovered in the process that the angular version matter. The plunk above use [email protected] and [email protected].

Angular/ui-router 1.3.1 1.2.25 1.2.26
0.2.11 OK OK OK
0.2.12 OK FAIL FAIL

@gfreeau
Copy link
Author

gfreeau commented Nov 20, 2014

http://plnkr.co/edit/OmVLsTIms86ka7hPXIeK

Navigate to /#/one/two?startDate=2014-11-17

@christopherthielen
Copy link
Contributor

@gfreeau @oliamb thanks I'll take a look

@gfreeau
Copy link
Author

gfreeau commented Nov 20, 2014

@christopherthielen added another test case, showing a default value of an empty string. I believe this is actually the issue reported in #1564

@samiconductor
Copy link

I ran into a similar problem when I was trying to clear out a parameter by setting it to null in the params arg of $state.go. The exception I'm getting is TypeError: Cannot read property 'to' of undefined and the stack trace points to this line (ui-router/release/angular-ui-router.js:1587:101)

var replacement = map(filter(self.replace, hasReplaceVal(value)), function(obj) { return obj.to; });

In the above line, self.replace is set to the default [{"from":""},{"from":null}] by the Param constructor. And when you filter it

// value = null
filter(self.replace, hasReplaceVal(value))
// [undefined, {"from":null}]

hasReplaceVal matches the second object since obj.from is null in this case. The map function causes the error since

map([undefined, {"from":null}], function(obj) { return obj.to; });
// undefined.to throws TypeError

The problem is the filter impl:

function filter(collection, callback) {
  var result = isArray(collection) ? [] : {};
  forEach(collection, function(val, i) {
    if (callback(val, i))
      // i in the case above is 1 which leaves 0 empty
      result[i] = val;
  });
  return result;
}

It should be more like:

function filter(collection, callback) {
  var result = isArray(collection) ? [] : {};
  forEach(collection, function(val, i) {
    if (callback(val, i)) {
      if (isArray(result)) {
        result.push(val);
      } else {
        result[i] = val;
      }
    }
  });
  return result;
}

I think this will fix the default param issue as well.

@christopherthielen
Copy link
Contributor

@samiconductor thanks

@SharmaAjay9
Copy link

One negative effect of this issue with ui-sref/ui-sref-active:

// for this state
.state('things', {
  url: '/thing/?filterBy',
  templateUrl: 'views/things.html',
  controller: 'thingsCtrl'
})

Up to 0.2.12, this code would work as expected. Either All or Red is active.

<!-- this html raise javascript errors in 0.2.12 -->
<a href ui-sref="things({filterBy: null})" ui-sref-active="active">All</a>
<a href ui-sref="things({filterBy: 'red'})" ui-sref-active="active">Red</a>

Since version 0.2.12, you have to write the code in one of the following form, which all behave in a wrong way (either all is always active or never active).

omitting the key

<!-- this html will not behave as expected -->
<a href ui-sref="things()">All</a>
<a href ui-sref="things({filterBy: 'red'})">Red</a>

using undefined as the key value

<!-- this html will not behave as expected -->
<a href ui-sref="things({filterBy: undefined})">All</a> <!-- yes this is equivalent to the previous -->
<a href ui-sref="things({filterBy: 'red'})">Red</a>

or using an empty string

<!-- this html will not behave as expected -->
<a href ui-sref="things({filterBy: ''})">All</a> <!-- In this case 'all' is never active -->
<a href ui-sref="things({filterBy: 'red'})">Red</a>

This worked for me thanks

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

No branches or pull requests

7 participants