Skip to content
This repository has been archived by the owner on Oct 26, 2018. It is now read-only.

Calling routerReducer() with no args crashes #349

Closed
mindjuice opened this issue Apr 5, 2016 · 5 comments · Fixed by #350
Closed

Calling routerReducer() with no args crashes #349

mindjuice opened this issue Apr 5, 2016 · 5 comments · Fixed by #350

Comments

@mindjuice
Copy link
Contributor

If you call routerReducer() with no arguments to get back the initial state, it crashes with the following error:

reducer.js: Uncaught TypeError: Cannot read property 'type' of undefined

The generated code looks like this:

function routerReducer() {
  var state = arguments.length <= 0 || arguments[0] === undefined ? initialState : arguments[0];
  var _ref = arguments[1];
  var type = _ref.type;       // <==== _ref is undefined here and _ref.type causes the error
  var payload = _ref.payload;

  if (type === LOCATION_CHANGE) {
    return _extends({}, state, { locationBeforeTransitions: payload });
  }

  return state;
}

This can be fixed in the original code by adding a default parameter for the action parameter:

export function routerReducer(state = initialState, { type, payload } = {}) {

It made me wonder though what the correct behavior of Babel should be here. Should it provide safe destructuring or still leave that up to your code? I checked Traceur though and it behaves the same way, so perhaps that's not part of the spec, but it seems dangerous.

mindjuice added a commit to mindjuice/react-router-redux that referenced this issue Apr 5, 2016
Fixes reactjs#349 in parent

Also fixed a typo in the function comment.
@mindjuice mindjuice changed the title Calling routeReducer() with no args crashes Calling routerReducer() with no args crashes Apr 5, 2016
@timdorr
Copy link
Member

timdorr commented Apr 6, 2016

Babel is handling destructuring properly. nil/null/undefined handling is a normal problem with just about every weak typed language out there. Even when it's natively supported by the VM, it still means you're trying to access a property of undefined. I'm not sure what the spec says about that, but I imagine it handles it with an error as well.

timdorr pushed a commit that referenced this issue Apr 6, 2016
Fixes #349 in parent

Also fixed a typo in the function comment.
@mindjuice
Copy link
Contributor Author

Thanks for the merge.

I can understand the undefined handling in the normal case where you are writing the code to dereference explicitly, but with destructuring, that code is written behind the scenes and it's much easier to make a mistake IMO. Oh well...guess I need to join TC39! 😄

I tried reading the spec, but wow...not an easy doc to read, even just the sections on destructuring!

http://www.ecma-international.org/ecma-262/6.0/

@mindjuice
Copy link
Contributor Author

@timdorr Any idea when we can get a new release with this fix in it?

Thanks!

@timdorr
Copy link
Member

timdorr commented Apr 6, 2016

I'll try to get something out tonight. For now, you can install straight from github.

@mindjuice
Copy link
Contributor Author

Thanks! Much appreciated Tim!

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

Successfully merging a pull request may close this issue.

2 participants