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

Is README.md correct for defining handler lookup function? #264

Open
efunneko opened this issue Oct 11, 2018 · 7 comments
Open

Is README.md correct for defining handler lookup function? #264

efunneko opened this issue Oct 11, 2018 · 7 comments

Comments

@efunneko
Copy link

The README.md file says:

  router.getHandler = function(name) {
    return myHandlers[name];
  };

This doesn't seem to do anything, but having dug into the library it seems to error on this.router.getRoute() not being defined. Should the README.md say that router.getRoute needs to be set instead?

With that said, I did change my code to set getRoute, which did stop the error, but my handler is still not being called. Is there a working example anywhere that I could follow?

@efunneko
Copy link
Author

efunneko commented Oct 11, 2018

Small amount of debugging later and it seems to be failing now at:

                this.triggerEvent(this.currentRouteInfos, true, 'didTransition', []);

where this.triggerEvent is undefined (this is a Router). I can't find anywhere in the source that ever sets router.triggerEvent. Note that I am currently using 6.0.0. Is 6.0.0 the correct version to use?

@fsauter
Copy link

fsauter commented Oct 18, 2018

I just stumbled upon this lib and tried to give it a shot. For me it works with router.getRoute instead of router.getHandler.

After a quick search I found the hint here: https://github.com/tildeio/router.js/blob/master/tests/query_params_test.ts#L65

@rwjblue
Copy link
Collaborator

rwjblue commented Oct 18, 2018

Yes, you are correct. We recently did some terminology cleanup, and apparently missed some in the README.md. Sorry about that.

Would love a PR fixing if someone has the time...

@fsauter
Copy link

fsauter commented Oct 18, 2018

Also took me a while: I had to implement router.triggerEvent = function() {}; -> otherwise it would not execute the example on README.md. @efunneko I guess you would have to do the same.

Found the hint here: https://github.com/tildeio/router.js/blob/master/tests/test_helpers.ts#L188

@efunneko
Copy link
Author

Thanks @fsauter for the pointers. I will try implementing the triggerEvent to move forward a bit.

@dakt
Copy link

dakt commented Apr 27, 2019

Come on guys, why having a README if it's misleading? I'm ditching this lib, can't rely on something that is half baked.

@Quasar-Kim
Copy link
Contributor

It seems like abstract methods defined in router.ts must be implemented. (#248)
I think this behavior should be documented.

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

No branches or pull requests

5 participants