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

stateError and stateChangeError events should be combined #62

Open
ArtskydJ opened this issue Apr 1, 2016 · 3 comments
Open

stateError and stateChangeError events should be combined #62

ArtskydJ opened this issue Apr 1, 2016 · 3 comments

Comments

@ArtskydJ
Copy link
Collaborator

ArtskydJ commented Apr 1, 2016

No description provided.

@TehShrike
Copy link
Owner

Yeah, it's really confusing what the difference between them is, and worse, I don't think the tests do a good job of asserting that they do the things specific to them (why is it a stateError and not a stateChangeError to navigate to a nonexistent state?).

A couple things:

  1. is there any real distinction to be found?
    1. Is it notable that some error events only happen between state changes starting and ending? I think that might be the current logic. Maybe this could be indicated by just a boolean between state changes.
    2. how significant is the difference between an error happening in a user-land function (like the provided activate function, or event handlers) and some other error happening (not able to turn a state name into a route when go is called or not being able to find a matching state when evaluateCurrentRoute is called)
  2. are any of these state routing issues serious enough to emit actual error events?

@TehShrike
Copy link
Owner

current stateError events:

handleError('stateError', err)
- I think this would only happen if a stateChangeAttempt listener threw an error. This should probably just be thrown in another tick instead of being emitted as a state-router error.

Same for

handleError('stateError', e)
which is just catching possible errors in event listener code.

For

handleError('stateError', err)
in evaluateCurrentRoute, the only place I see that could be causing that error would be if prototypalStateHolder.guaranteeAllStatesExist threw, which means that someone tried to navigate to a state that doesn't exist.

Maybe that should just throw straight out as well? It seems like a different kind of "not found" than the traditional one I'd imagine existing, which would be "there was no matching route for the url in the location bar".

@ArtskydJ
Copy link
Collaborator Author

ArtskydJ commented Jun 24, 2016

I would expect to be directed to the hashBrownRouter.setDefault (404) state if the state doesn't exist.

If there wasn't a hash, I would expect to go to the fallbackState.

If I call state.go('this.route.does.not.exist'), I would expect to have an error thrown.

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

No branches or pull requests

2 participants