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

NavigationActions have lost their `toString()` implementations #4072

Closed
jamesreggio opened this Issue Apr 26, 2018 · 2 comments

Comments

Projects
None yet
3 participants
@jamesreggio
Contributor

jamesreggio commented Apr 26, 2018

@ericvicenti, in #3619 you deleted the tests that confirm that each NavigationAction function has a toString() that returns its action name.

I had added these back in #2260, and the justifications remain. I'm going to hack them back in for my own usage, and I'd be happy to submit a PR if this was merely an oversight.

If you have philosophical disagreements with them, let's discuss — but, at minimum, you should note it as a breaking change and remove it from the documentation here (search for toString()).

Current Behavior

import {NavigationActions} from 'react-navigation';
NavigationActions.navigate.toString() !== 'Navigation/NAVIGATE';

Expected Behavior

NavigationActions.navigate.toString() === 'Navigation/NAVIGATE';

...and so on, for each action creator.

Your Environment

software version
react-navigation 2.0.0-rc.4
@ericvicenti

This comment has been minimized.

Contributor

ericvicenti commented Apr 26, 2018

Is this toString behavior in the docs? If so, let’s put it back. But honestly I’d rather just keep the actions simple and report this as a breaking change.

@jamesreggio

This comment has been minimized.

Contributor

jamesreggio commented Apr 26, 2018

Yeah, the toString() behavior is in the docs. It's still a common pattern to match against the action-creator function with various Redux libraries.

I'll send you a PR tomorrow.

@brentvatne brentvatne closed this Jun 7, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment