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

Update combineReducers.js #2187

Merged
merged 1 commit into from Jan 9, 2017

Conversation

3 participants
@slorber
Contributor

slorber commented Jan 6, 2017

This may not seem obvious to many of us, but we can write

const bootstrapDataReducer = (state = null, event) => {
  switch (event.name) {
    case APPLICATION_STARTED:
      return event.data.bootstrapData;
    default:
      return state;
  }
};

instead of

const bootstrapDataReducer = (state = {}, event) => {
  switch (event.name) {
    case APPLICATION_STARTED:
      return event.data.bootstrapData;
    default:
      return state;
  }
};

Assigning an empty object as initial state looks to me an anti-pattern, yet many people tend to do this to satisfy Redux

I think it would give a better developer experience to document the ability to use "null" instead of "undefined" as reducer state in error messages

Update combineReducers.js
This may not seem obvious to many of us, but we can write

```
const bootstrapDataReducer = (state = null, event) => {
  switch (event.name) {
    case APPLICATION_STARTED:
      return event.data.bootstrapData;
    default:
      return state;
  }
};
```
instead of 

```
const bootstrapDataReducer = (state = {}, event) => {
  switch (event.name) {
    case APPLICATION_STARTED:
      return event.data.bootstrapData;
    default:
      return state;
  }
};
```

Assigning an empty object as initial state looks to me an anti-pattern, yet many people tend to do this to satisfy Redux

I think it would give a better developer experience to document the ability to use "null" instead of "undefined" as reducer state in error messages
@markerikson

This comment has been minimized.

Contributor

markerikson commented Jan 6, 2017

Erm... not sure I follow what you're saying here. Can you give some more details on the background and intent?

@slorber

This comment has been minimized.

Contributor

slorber commented Jan 6, 2017

@markerikson I mean that sometimes you want to create a reducer, but you don't want it to have an initial value assigned. And current error message does not really explain that "null" can be used for such usecase, it only explains that undefined is an illegal value.

If we had types, the state of my reducer would be an Option and its initial value would be None, but we don't so null seems appropriate to assign no value to this reducer's state

In JS/Scala mix I would write something like:

const bootstrapDataReducer = (state: Option[BootstrapData] = None, event) => {
  switch (event.name) {
    case APPLICATION_STARTED:
      return Some(event.data.bootstrapData);
    default:
      return state;
  }
};
@timdorr

This comment has been minimized.

Member

timdorr commented Jan 9, 2017

More detailed error messaging isn't a bad thing. Thanks!

@timdorr timdorr merged commit 920e72e into reduxjs:master Jan 9, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

seantcoyote added a commit to seantcoyote/redux that referenced this pull request Jan 14, 2018

Update combineReducers.js (reduxjs#2187)
This may not seem obvious to many of us, but we can write

```
const bootstrapDataReducer = (state = null, event) => {
  switch (event.name) {
    case APPLICATION_STARTED:
      return event.data.bootstrapData;
    default:
      return state;
  }
};
```
instead of 

```
const bootstrapDataReducer = (state = {}, event) => {
  switch (event.name) {
    case APPLICATION_STARTED:
      return event.data.bootstrapData;
    default:
      return state;
  }
};
```

Assigning an empty object as initial state looks to me an anti-pattern, yet many people tend to do this to satisfy Redux

I think it would give a better developer experience to document the ability to use "null" instead of "undefined" as reducer state in error messages
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment