Skip to content

Non-FSA actions cause handleAction reudcers to emit confusing error messages #164

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

Closed
noahsilas opened this issue Nov 18, 2016 · 6 comments · Fixed by #168
Closed

Non-FSA actions cause handleAction reudcers to emit confusing error messages #164

noahsilas opened this issue Nov 18, 2016 · 6 comments · Fixed by #168

Comments

@noahsilas
Copy link

In #141 an invariant checks that each action is an FSA before attempting to process it. When attempting to upgrade to 1.0.0, this causes issues in a large app I am working on where our more recent actions follow the FSA pattern, but older actions do not. When those older actions were triggered, the invariant caught them and prevented the reducers that are actually intended to process that type of action from receiving it.

This is at odds with allowing larger apps to migrate incrementally; we need to update all of our actions / reducers to upgrade.

Further, the error message generated by #141 was actually misleading; the actions had a type attribute, which is a stronger redux convention than FSA, but had attributes that aren't on the FSA whitelist (https://github.com/acdlite/flux-standard-action/blob/master/src/index.js).

It seems like there are a couple decisions to be made:

  1. Should using this package require that all of your actions be FSA?
  2. Should using this package require that all of your actions have a type?

If the answer to (1) is "yes", then we can close this issue (although we should probably document this in the README).

If the answer to (1) is "no", then I can use an answer to (2) to write a PR. Let me know what y'all think.

IMO, reducers created by handleAction should only complain if an action is emitted with a type that they are listening for that is not an FSA. That opinion seems to be at odds with this request, which makes it sound like someone encountered an incident where they were emitting an action without a type that was hard to diagnose. In that scenario, it seems like the action wasn't created via createAction, and so I sort of expect that onus to fall on the developer (although I could certainly be convinced that there are better options available).

@yangmillstheory
Copy link
Contributor

Hi,

Sorry for the inconvenience. Following semver, 1.0.0 does include breaking changes, so it's expected that application code will have to change to accommodate, although an incremental upgrade path would be ideal. Sorry for not providing that.

To clarify, the answer to 1 is yes, I believe the original intent of this library is to provide utilities for FSA.

Thus, the action items seem to be:

@timche
Copy link
Member

timche commented Nov 19, 2016

With v1.0.0 we are trying to make this library more user-friendly with throwing errors if something is wrong, so debugging is much easier.

This library is primarily designed for Flux Standard Actions since the beginning, so it should only work with FSA:

An action MUST

  • be a plain JavaScript object.
  • have a type property.

@noahsilas
Copy link
Author

Just to be entirely clear; while I expect the reducers created via handleAction to only handle FSA, I don't expect that having separate, non-FSA-style actions in my app to cause an invariant error. This slows adoption of this library, because I can't start by adding a few redux-action created actions + reducers, but instead need to migrate the entire app onto FSA actions.

@timche: The criteria you laid out aren't sufficient to avoid the error. For example, imagine the following action creator already exists in your app:

export function sessionStarted(user) {
  return {
    type: 'SESSION_STARTED',
    user: user,
  };
}

Because the user key isn't in the FSA whitelist, this is not an FSA, and so can't be passed in to a reducer created by redux-actions, even if the reducer would go on to ignore this event because it isn't of a type the reducer handles.

In this sense, the placement of the invariant sort of "infects" the entire redux store with a requirement for FSA actions. While this might be desirable for new projects, it does make adoption of the library for existing apps a bit of a chore.

@tricoder42
Copy link

Example: redux-form@^5.0.0 doesn't use FSA-compliant actions even though all has type.

timche added a commit that referenced this issue Nov 21, 2016
With #141 we made other libraries incompatible with redux-actions and results to an unexpected behavior. If an action is not a FSA, handleAction should not handle it and just return it to the next reducer. This fix will remove the FSA check.\n\nCloses #164, #167
timche added a commit that referenced this issue Nov 21, 2016
With #141 we made other libraries incompatible with redux-actions and results to an unexpected behavior. If an action is not a FSA, handleAction should not handle it and just return it to the next reducer. This fix will remove the FSA check.

Closes #164, #167
timche added a commit that referenced this issue Nov 21, 2016
With #141 we have made other libraries incompatible with redux-actions and it resulted to an unexpected behavior (#164, #167). If an action is not a FSA, handleAction should not handle it and just pass it to the next reducer. This fix will remove the FSA check to support Non-FSA.

Closes #164, #167
timche added a commit that referenced this issue Nov 21, 2016
With #141 we have made other libraries incompatible with redux-actions and it resulted to an unexpected behavior (#164, #167). If an action is not a FSA, handleAction should not handle it and just pass it to the next reducer. This fix will remove the FSA check to support Non-FSA.

Closes #164, #167
@timche
Copy link
Member

timche commented Nov 21, 2016

Thanks for referencing to other libraries. We have opened #168 to resolve this issue.

timche added a commit that referenced this issue Nov 21, 2016
With #141 we have made other libraries incompatible with redux-actions and it resulted to an unexpected behavior (#164, #167). If an action is not a FSA, handleAction should not handle it and just pass it to the next reducer. This fix will remove the FSA check to support Non-FSA.

Closes #164, #167
@timche
Copy link
Member

timche commented Nov 21, 2016

We have published v1.0.1. Let us know if you still have any issues, thanks :)

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

Successfully merging a pull request may close this issue.

4 participants