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

takeEvery doesn't work when action type is a Symbol #246

Closed
Zacqary opened this issue Apr 5, 2016 · 8 comments
Closed

takeEvery doesn't work when action type is a Symbol #246

Zacqary opened this issue Apr 5, 2016 · 8 comments
Labels

Comments

@Zacqary
Copy link

Zacqary commented Apr 5, 2016

When dispatching actions with a type equal to a Symbol, takeEvery throws TypeError: Cannot convert a Symbol value to a string.

const ACTION_TYPE = Symbol("ACTION_TYPE Debug String");
// Inside a saga
yield* takeEvery(ACTION_TYPE, sagaToYield); // Throws TypeError

However, using take works fine.

for (;;) {
    yield take(ACTION_TYPE);
    // Yield results...
}
@yelouafi
Copy link
Member

yelouafi commented Apr 6, 2016

The issue seems from here. String interpolation doesn't work for symbols

@yelouafi yelouafi added the bug label Apr 6, 2016
@yelouafi
Copy link
Member

fixed by c9c63d9

@jscinoz
Copy link
Contributor

jscinoz commented May 31, 2016

Seems to still be an issue when pattern is an array of symbols; the following line throws with TypeError: Cannot convert a Symbol value to a string:

https://github.com/yelouafi/redux-saga/blob/master/src/internal/sagaHelpers.js#L64

@bvalosek
Copy link

bvalosek commented Jun 21, 2016

(highly related question to this thread)

When using a action creator function with a toString property, currently that is preventing take from working (very similar to this issue).

eg

const login = credentials => {type: 'LOGIN', payload: credentials};
login.toString = () => 'LOGIN';


...

dispatch(login(creds));

...

take(login)

Or more specifically: https://github.com/acdlite/redux-actions

Is ensuring that take(pattern) calls String(pattern) like the above fixes something we could have as well?

@yelouafi
Copy link
Member

@bvalosek What error do you get?

@bvalosek
Copy link

I realized that this can't be implemented without breaking BC with your API.

I was asking if, when passing in a function that overrides toString that it could be cast to a string when running the matcher... but that would break the current expectations when calling take(fn) (as it serves as a predicate filter in this case).

https://github.com/yelouafi/redux-saga/blob/master/src/internal/proc.js#L17

What I am doing now is that i've made a utility const takes = p => take(String(p)) to use in all the places where i have an action creator function that returns the action type when cast to a string. If you're open to having this be a built-in Effect then I can open a PR.

@thezanke
Copy link
Contributor

thezanke commented Dec 29, 2016

I found my way here looking to see if there's a way to automatically call .toString() on a redux-actions actionCreator. If this was possible I could completely drop my action type constants file (since I don't need them for reducers with redux-actions).

I opened a new issue here with a suggested solution.

@iMoses
Copy link
Contributor

iMoses commented Jan 21, 2017

Was this issue solved?

I'm new to redux-saga and I couldn't make it work until I figured that the problem was my types were Symbols. I get no errors, no warnings, but the middleware doesn't catch any of my actions as long as the type is a Symbol.

Are Symbols not supported or is this a bug?

The problem seems to originate in the proc.js file at the default matcher, in which the following comparison takes place:

input.type === String(pattern)

This could be easily fixed by checking if the type is a symbol and if so we shouldn't turn it into a string.

input.type === (typeof input.type === 'symbol' ? pattern : String(pattern))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants