-
Notifications
You must be signed in to change notification settings - Fork 142
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
Serializable? #9
Comments
Yes, I think that's a sensible requirement. However, we should distinguish between "in-flight" actions and "processed" actions that are ready for consumption by a Flux store, reducer, whatever. We should come up with a good term for those, as well. That way this is still a legal "in-flight" FSA action: {
type: 'FETCH_DATA'
payload: somePromise
} that will be turned into proper "processed" FSA actions by middleware or some other library code (trying to stay framework agnostic, remember 😄): {
type: 'FETCH_DATA'
payload: { id, data, ..stuff }
} |
Oh, I forgot that right now the spec says the payload of a failed action should be an error object, which isn't serializable. Not a problem, we can specify that the {
type: 'FETCH_DATA',
error: true,
payload: {
message: "Something's gone wrong",
fileName: 'index.js',
lineNumber: 123
}
} This is a "SHOULD," though, not a "MUST." The only "MUST" is that it is serializable. |
@acdlite Defining a point where an action must comply with certain criteria is important. It might vary depending on criteria. |
I'm bumping into this writing debug tools as well. Sending actions over https://developer.mozilla.org/en-US/docs/Web/Guide/API/DOM/The_structured_clone_algorithm The browser will gripe at you if you try to |
How about a dev-only middleware that you'd put at the very end, and that warns you when your actions aren't FSA-compliant (or serializable for that matter)? |
I think the warn middleware would be handy (in its own repo). Would also be useful to expose a utility method like |
Just hit this. It seems like serializing the Error object is the tricky part. The convention proposed be @acdlite makes sense to me, e.g.:
since this mimics the Error object structure: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error Mentioning this convention in the docs would be welcome. |
I implemented an FSA serialization library: https://github.com/rstuven/flux-standard-action-json I tried to be strict where the FSA spec is (throwing an error when the action to be serialized/deserialized is not compliant, ie. I'll include a validation for something like Hope you find it useful. :) |
Weirdly I just saw this issue, and have just published something like what @gaearon suggests for linting actions: https://github.com/maxmechanic/redux-fsa-linter |
Are error objects suppose to be only for clientside errors? Cause given your example: {
type: 'FETCH_DATA',
error: true,
payload: {
message: "Something's gone wrong",
fileName: 'index.js',
lineNumber: 123
}
} If the error was produced by the server, I'm not sure you have any meaningful fileName and lineNumber. e.g. if Also-- |
Should FSA mandate actions to be serializable? I think it's a good requirement (prevents people from passing callbacks inside actions). It's also great for the tooling, as it's easy to build Flux over websockets, or a persistent debug session solution, without worrying that something might get lost.
I propose the following changes:
invariant(isDeepEqual(action, JSON.parse(JSON.stringify(action))))
type
must be a String, not a SymbolWe could then write a middleware that checks actions for compliance in development.
The text was updated successfully, but these errors were encountered: