-
-
Notifications
You must be signed in to change notification settings - Fork 82
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
Action validator throws on an action without a 'name' property (like the ones dispatched by the 'redux-devtools') #3
Comments
I don't think it's a Redux issue. "Type" is an established convention, at least in Flux world, that's where most of our conventions come from. There need to be very good reasons to break these conventions, and more so after we shipped 1.0 and lots of code is already written using "type". I think that's something for redux-immutable to reconsider. |
I have rewritten the test to check if action has @gaearon Is right in a sense that:
This is also a convention defined in "flux-standard-action" spec. However, I have argued that "type" name as a convention does not make semantic sense. "redux-immutable" implements the https://github.com/gajus/canonical-reducer-composition/ spec. Read the action object spec for more details. I will update the main "canonical-reducer-composition" description to include explanation why to use this "unconventional" spec over what has been established in flux land, but the short answer is: "canonical-reducer-composition" tries to be
In the mean time, I am working on dev-tools that will work with both "redux" and "redux-immutable". |
@gajus This also breaks the logger middleware (it shows 'undefined' instead of the action type) At least make it configurable where default could be 'name', (I guess we can just give the Thanks. |
@asaf Just an idea, but why not have a function at the top of the middleware chain that maps "type" to "name" in every action it receives? This way anyone can use whatever name they choose if they don't like "name" convention. I can write a quick middleware package for that. The same middleware function could map other properties that do not meet the standard, such as "payload". Does that sound like a good solution? |
A simplified version would look like this: store = applyMiddleware(
mapper.fromCCAtoFSA,
/* all middleware that needs to be FSA compatible */
logger,
mapper.fromFSAtoCCA
)(createStore)(reducer, state); Where the mapper is something along the lines of: let fromCCAtoFSA,
fromFSAtoCCA;
fromCCAtoFSA = (store) => (next) => (action) => {
next({
type: action.name,
payload: action.data
});
};
fromFSAtoCCA = (store) => (next) => (action) => {
next({
name: action.type,
data: action.payload
});
};
export default {
fromCCAtoFSA,
fromFSAtoCCA
}; |
@gajus 👍 I like this convention-conversion stuff. |
Ok. Will research certain aspects of the conversion (such as Error mapping) and release a package. Should be before the end of the day. |
The solution is to use middleware for converting between the two standards. |
@gajus This is still problematic, as @gaearon mentioned, "lots of code is already written using "type"", this also means reducers are being written in the community and expecting type, conversion to 'name' won't satisfy, IMO, If you aim CCA out of the redux community you can diverge, but if you aim for Redux, you should be compatible with its standards otherwise community code will break. |
I use CRC as well as CCA in all the projects that I develop. I am sharing the CRC spec. I am sharing some libraries with the open source world and provide a method of bridging the different conventions. Sounds fair to me. |
The "Canonical Reducer Composition" validator checks every action object and throws if a
name
property is missing. Butredux
uses another convention – it usestype
, notname
, so no action object has a propername
in it by default.The core
@@redux/INIT
action is worked around and ignored inredux-immutable
, but theredux-devtools
use another action called@@INIT
which isn't.I'm not sure if this is an issue of
redux-immutable
or theredux
itself, but hopefully you guys will figure it out. CC @gajus @gaearonFor now I'm willing to uninstall
redux-immutable
rather thanredux-devtools
(until the issue is fixed somehow).The text was updated successfully, but these errors were encountered: