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

What is the reasoning for making redux validate action object? #1360

Closed
gajus opened this issue Feb 3, 2016 · 3 comments
Closed

What is the reasoning for making redux validate action object? #1360

gajus opened this issue Feb 3, 2016 · 3 comments

Comments

@gajus
Copy link
Contributor

gajus commented Feb 3, 2016

https://github.com/rackt/redux/blob/v3.2.1/src/createStore.js#L156-L161

if (typeof action.type === 'undefined') {
    throw new Error(
        'Actions may not have an undefined "type" property. ' +
        'Have you misspelled a constant?'
    )
}

There is no reason for Redux to perform this validation. Redux must only concern itself with the actions that it dispatches (namely, ActionTypes. INIT).

The workaround that is used at the moment (e.g. projects that implement https://github.com/gajus/canonical-reducer-composition) is to set action.type to a dummy property only to satisfy this single condition, which appears both a hack and unnecessary.

I'd like to propose to remove this condition.

@gaearon
Copy link
Contributor

gaearon commented Feb 3, 2016

This decision was taken because way too many people had problems with reducers not handling the actions. Turns out, usually it happened due to a typo when importing a constant in the reducer file:

import { INRCEMENT } from './actions'

Of course the matching case never hits, but beginners are often puzzled by this because typos can be hard to spot, and most bundlers don’t analyze dependencies to warn you about missing imports yet.

We felt this is a good enough reason to choose a single convention (in our case, the most popular one is the one that comes from Flux: type property). I understand not everyone is happy with the name, but you can’t please everyone.

The developer efficiency that we get from this tradeoff seems worthy to me.

The only reason Canonical Composition does not fit this pattern is because it insists on using name property for the same thing. We had to choose a name to solve a problem, and we picked the most popular one. So either you’ll need to work around it, or adopt type like everyone else :-).

Previous discussions:

We don’t plan to change this.

@gaearon gaearon closed this as completed Feb 3, 2016
@gajus
Copy link
Contributor Author

gajus commented Feb 3, 2016

Sounds a lot like a responsibility of a dev middleware (validateAction) to me, esp. given that this should only be done in development setup.

I respect your decision though.

On Feb 3, 2016, at 12:34, Dan Abramov notifications@github.com wrote:

This decision was taken because way too many people had problems with reducers not handling the actions. Turns out, usually it happened due to a typo when importing a constant in the reducer file:

import { INRCEMENT } from './actions'
Of course the matching case never hits, but beginners are often puzzled by this because typos can be hard to spot, and most bundlers don’t analyze dependencies to warn you about missing imports yet.

We felt this is a good enough reason to choose a single convention (in our case, the most popular one is the one that comes from Flux: type property). I understand not everyone is happy with the name, but you can’t please everyone.

The developer efficiency that we get from this tradeoff seems worthy to me.

The only reason Canonical Composition does not fit this pattern is because it insists on using name property for the same thing. We had to choose a name to solve a problem, and we picked the most popular one. So either you’ll need to work around it, or adopt type like everyone else :-).

Previous discussions:

#564
#541
We don’t plan to change this.


Reply to this email directly or view it on GitHub.

@gaearon
Copy link
Contributor

gaearon commented Feb 3, 2016

Sounds a lot like a responsibility of a dev middleware (validateAction) to me, esp. given that this should only be done in development setup.

The problem with this specific mistake is you don’t know you need that middleware until you’ve spent an hour on this and it’s too late :-)

I know it’s opinionated but I think we made the right tradeoff here.

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

2 participants