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

Allow es6 symbols to be used as action types #295

Merged
merged 2 commits into from
Jul 22, 2015
Merged

Allow es6 symbols to be used as action types #295

merged 2 commits into from
Jul 22, 2015

Conversation

jhewlett
Copy link
Contributor

Symbols are a good way to prevent naming collisions of action types.

However, if you attempt to use a symbol as an action type, getErrorMessage inside combineReducers will throw an error on line 13 because symbols can't be implicitly converted to a string. I can reproduce this in at least Chrome and Firefox.

Strangely enough, I could not reproduce this in the tests -- it looks like babel is implicitly converting the symbol to a string, contrary to the es6 spec as I understand it. I added a test anyway to verify the use case.

I understand Redux probably wasn't designed to be used with symbols as action types, but this seems like an easy way to add support for them.

I'm also open to other solutions or workarounds.

- Symbols are a good way to ensure uniqueness of your action types
- In Chrome and Firefox, a symbol inside of a template literal throws an
  error. Making the string conversion explicit gets rid of the error
@gaearon
Copy link
Contributor

gaearon commented Jul 22, 2015

I'm not sure we want to allow this but you're right that, even if we throw, we shouldn't throw inside the helper method..

The reason I would rather enforce string action type is because you can't serialize Symbols. The whole bunch of devtools and plugins would be unavailable to you. It's so easy to make this mistake without realizing the consequences that I'd rather enforce strings.

Thoughts?

@gaearon gaearon mentioned this pull request Jul 22, 2015
13 tasks
gaearon added a commit that referenced this pull request Jul 22, 2015
Allow es6 symbols to be used as action types
@gaearon gaearon merged commit 8bb94c7 into reduxjs:breaking-changes-1.0 Jul 22, 2015
@gaearon
Copy link
Contributor

gaearon commented Jul 22, 2015

👍

While we won't encourage people to use Symbols (and will even actively discourage doing so) I don't think we should be opinionated here, and definitely we shouldn't crash inside the warning method.

@jhewlett
Copy link
Contributor Author

Yeah, that makes perfect sense.

I wonder if it's even worth keeping the test. Like I said, the test never failed before I made the change, presumably because babel has different behavior than the browsers that support Symbols.

@goatslacker
Copy link

You can serialize Symbols by calling toString() on them. You will need to use Symbol.keyFor though if you want actions to be replayable.

@jhewlett
Copy link
Contributor Author

@goatslacker The problem with toString is that Symbol('test').toString() and Symbol('test').toString() (two unique symbols) both serialize to 'Symbol(test)' (at least in Chrome and FF). How would you distinguish between the two? Furthermore, I don't see how you would deserialize them back into a symbol at that point.

@goatslacker
Copy link

That's why I mentioned you have to use Symbol.keyFor.

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.

3 participants