Write tests/fix "not found" behavior for evaluateCurrentRoute

[TehShrike]

Right now evaluateCurrentRoute only checks "is there any route" before sending you to a default state.

It would probably be more correct to check "is the route empty?" and if so, send you to that default state, and if not, emit a notFound event or send you to a 404-not-found page.

Thoughts?

[TehShrike]

Example case:

• when you open the browser with a url of #/legitimate-state and call evaluateCurrentRoute('default-state'), that legitimate state should be loaded.
• when you open the browser with a url of # and call evaluateCurrentRoute('default-state'), the default-state should be loaded.
• when you open the browser with a url of #/some-invalid-route and call evaluateCurrentRoute('default-state'), default-state is loaded, but it probably shouldn't be.

[AgentChris]

i've tried to solve this issues in 2 ways (i'm using it with Riotjs).
First solution:
on the event : routeNotFound(route, parameters) i will call have this code:

stateRouter.on("routeNotFound",function(path,params){ stateRouter.go("route_not_found") })

The code above will load the state: route_not_found , which is defined below:

{ name: 'route_not_found', route: '/not-found-page', template: 'not-found-page', activate: function (context) { } }

But there is a problem: the current route now is 'not-found-page' , and if press the back button it will go a the route "#/some-invalid-route" and then again will load the 'route_not_found' page

I've tried to fix this, by removing the route atribute, but this will load the default state:

{ name: 'route_not_found', template: 'not-found-page', activate: function (context) { } }

My second solution:

on the event : routeNotFound(route, parameters) i will call have this code:

stateRouter.on("routeNotFound",function(path,params){ riot.mount("body","not-found-page') })

This will load the proper component but when i hit the back button it will not load the previous state.
I think this problems occurs because I am not using stateRouter.go, so the abstract-router thinks the state is loaded.

[AgentChris]

I've been thinking about two solution.
In the first solution maybe abstract-router could provide a way to load a specific state when there is an invalid route #/some-invalid-route , but keep the invalid url in browser, to keep the backbutton working properly.
The second solution is maybe re-working the route attribute, to not automatically redirect to the default page.
For the first solution there is a problem though, let's say you want to load different states that will define route_not_found type templates. For example in the login page maybe you a specific page for not_fount_page.
See the image below:

But let's say you are logged in the application, the you will want anoter not_found_page

[TehShrike]

You can get that first example to work reasonable if you pass in the replace option:

stateRouter.on("routeNotFound", function(path,params){
   stateRouter.go("route_not_found", { replace: true }) 
})

This will replace the invalid url with your route_not_found page in the browser history, and keep the back button from being broken.

---

"Not found"

Since writing this issue, and issue #62, I've thought a bit more about the difference between "404 not found" and other state change errors.

I think that any time an invalid url is visited (a url with no state matching the route), a full-screen 404 state should be displayed.

This would never happen when someone is clicking through your app, or if you're making programmatic state.go calls, because your links should all be built with makePath and makePath, along with navigating programmatically with state.go, all reference states, not urls.

If you call makePath or go with an invalid state name, the url will not change, so there's no need for the full-screen 404 not found page.

Now, trying to call makePath or go with an invalid state name should emit some kind of an error, but that would be up to the program to handle I think. The state router doesn't have the knowledge to be able to change the url when the programmer references a nonexistent state.

---

There are cases where it makes sense to show an error screen as a child state of other logged-in-menu states, but I don't think an invalid url in the location bar is one of them.

Those other cases aren't exposed awesomely by the abstract-state-router either, but I think the resolution of #62 will probably be "any time we knew what state to try to load, but weren't able to display it for some reason, you should be given the chance to display an error message to the user there or otherwise recover".

---

Next steps

I think the hash-brown-router needs to change its default/not found behavior to make this possible to fix in the abstract-state-router. I'll go write an issue for that now.

[AgentChris]

I couldn't fix the first example with replace option. The problem remains the same, also i don't need the invalid url to be replaced, it needs to stay there for the back button to work. I just need the content of the state of route_not_found not the route to change to "/not_found_route".

[AgentChris]

Now is working :) i found the problem, I've looked at your test and the proper code is:
stateRouter.go("route_not_found", {}, { replace: true })
not
stateRouter.go("route_not_found", { replace: true })

Thank you for the response ;). this is a useful library :)

[TehShrike]

oh man you're right >_< sorry about the misleading comment!

As I have the time, I'll start working on the code to display the 404 state without changing the url.

[TehShrike]

Okay, the [email protected] was published with the correct onNotFound/evaluateCurrent behavior.

After upgrading to this new version, the fix detailed in above comments can now be implemented.

[TehShrike]

My "not found" plan ran into some issues.

Right now, the abstract-state-router has an implied contract with consumers that 1. all states can be identified with a string, and 2. you can get a hold of every state/dom API by watching for the state create/reset events. (See https://github.com/TehShrike/asr-active-state-watcher)

I was going to implement this in a way that wouldn't involve a new identifier in the state names namespace, and wouldn't fire all of the regular events (like stateChangeStart). But now I'm afraid that if I implement it that way, it will make observing the router and its events much more difficult.

So I'm thinking of saying that the "not found" state will have a hardcoded state name of notfound, and any differences between transitioning to it and transitioning to a regular state (like say, the fact that the url route changes before you transition to a regular state) will be special cases in the code.

It's not what I wanted to do, but I think it will be necessary to keep my mental model for state change flow simple.

Any thoughts?

[TehShrike]

You know, I think you could implement a "not found" route by just adding a state with a wildcard (.*) as your first state...