Skip to content
This repository has been archived by the owner on Jun 5, 2020. It is now read-only.

Support for Symbols in action types #63

Merged
merged 4 commits into from
Jan 31, 2017
Merged

Conversation

zalmoxisus
Copy link
Collaborator

It just reimplements gaearon/redux-devtools-log-monitor#29 and also supports null as action type per gaearon/redux-devtools-log-monitor#55.

There's still an error in the demo, coming from redux-logger, as it doesn't support symbols.

@@ -234,7 +234,7 @@ export default connect(
payload: Array.from({ length: 10000 }).map((_, i) => i)
}),
addFunction: () => ({ type: 'ADD_FUNCTION' }),
addSymbol: () => ({ type: 'ADD_SYMBOL' }),
addSymbol: () => ({ type: Symbol('ADD_SYMBOL') }),
Copy link
Owner

Choose a reason for hiding this comment

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

Please add window. here - just to make eslint happy

@@ -92,6 +92,6 @@ export default {
) : state,
addFunction: (state=null, action) => action.type === 'ADD_FUNCTION' ?
{ f: FUNC } : state,
addSymbol: (state=null, action) => action.type === 'ADD_SYMBOL' ?
addSymbol: (state=null, action) => action.type === Symbol('ADD_SYMBOL') ?
Copy link
Owner

Choose a reason for hiding this comment

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

here too

@@ -48,7 +48,7 @@ export default class ActionListRow extends Component {
isInFuture && 'actionListFromFuture'
], isSelected, action)}>
<div {...styling(['actionListItemName', isSkipped && 'actionListItemNameSkipped'])}>
{action.type}
{action.type !== null && action.type.toString()}
Copy link
Owner

Choose a reason for hiding this comment

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

I suggest something like this:

action.type === null ?
  '<NULL>' :
  (action.type.toString() || '<EMPTY>')

Copy link
Collaborator Author

@zalmoxisus zalmoxisus Jan 8, 2017

Choose a reason for hiding this comment

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

That looks great. Will change now. Thanks for the review!

@zalmoxisus
Copy link
Collaborator Author

zalmoxisus commented Jan 28, 2017

Even though Redux warns when having undefined as action type, we should still check it here to avoid unexpected exceptions. For example, when persisting the state, a symbol will be added to localStorage as undefined, because JSON.stringify({ type: Symbol('BECOMES_UNDEFINED') }) === '{}'. It can be prevented with zalmoxisus/redux-devtools-instrument#14, but the parameter is optional and there could be other cases.

@alexkuz alexkuz merged commit 4c5f7af into alexkuz:master Jan 31, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants