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

Bypass action and meta creators if payload is an Error object #66

Merged
merged 3 commits into from
Jun 21, 2016

Conversation

andrewsuzuki
Copy link
Contributor

See #65.

@luisincrespo
Copy link

+1

@timche
Copy link
Member

timche commented May 15, 2016

Imo it shouldn't bypass the meta creators, unless there is a good reason to do so. If the meta creator is needed to handle the action, the action would maybe become useless without the meta information with this PR.

What do you think?

@@ -34,6 +34,7 @@
"chai": "^3.0.0",
"eslint": "^0.24.0",
"eslint-config-airbnb": "0.0.6",
"escope": "3.3.0",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this being used anywhere?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, should be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if I'm remembering correctly, but I think I needed to add that to bypass an issue with babel-eslint back when redux-actions was on babel 5. I'll remove it now.
estools/escope#99

@andrewsuzuki
Copy link
Contributor Author

@timche Just saw this. I agree, there could definitely be meta information associated with an error.

@andrewsuzuki
Copy link
Contributor Author

@timche Done, meta creator will still be run regardless of error now. Also my bad, should have rebased instead of merged.

@yangmillstheory
Copy link
Contributor

Is this still merge-able?

@timche
Copy link
Member

timche commented Jun 21, 2016

Sorry for the late reply, I was on vacation.

@timche timche merged commit b9d5eb7 into redux-utilities:master Jun 21, 2016
@timche
Copy link
Member

timche commented Jun 21, 2016

Merged, thanks for the changes :)

@kujon
Copy link

kujon commented Jul 11, 2016

It totally reverted #75

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 this pull request may close these issues.

5 participants