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

root path gives me the same behavior with * #56

Closed
ifig opened this issue Oct 24, 2016 · 17 comments
Closed

root path gives me the same behavior with * #56

ifig opened this issue Oct 24, 2016 · 17 comments

Comments

@ifig
Copy link

ifig commented Oct 24, 2016

I am having trouble with the following code : When I type the following url '/v1/a/b/c', I'd like to match the "not found" handler. Instead, I match the root path that I defined at last ('/'). It's like I would have defined it that way : '/*'.

var router = new Navigo('/v1');

router.on('modify', function () {
    setContent('modify');
})
.on('modify/:name', function (params, query) {
    setContent('name');
})
.on('/', function () {
    setContent('home');
})
router.notFound(function () {
    console.log("not found");
    // called when there is path specified but
    // there is no route matching
});
router.resolve();
@krasimir
Copy link
Owner

krasimir commented Oct 24, 2016

Can we try something quick. Can you please put the notFound call before the other routes definitions. Just after var router = new Navigo('/v1');.

@ifig
Copy link
Author

ifig commented Oct 24, 2016

Same result.

@krasimir
Copy link
Owner

Ok, then that's a bug. Thanks for creating the issue. I'll take care soon.

@ifig
Copy link
Author

ifig commented Oct 24, 2016

Thank you ! It's weird though, because no one opened the issue before, but it can't be that no one ran the same issue, when you usually define the home page on the "/" route.

@krasimir
Copy link
Owner

@ifig I just released 3.3.1 version. Can you please try it out. Should be fixing your issue. Here's is the test case that replicates your problem:

      var router = new Navigo('/v1');
      var notFoundHandler = sinon.spy();
      var modifyHandler = sinon.spy();
      var defaultHandler = sinon.spy();

      router
      .notFound(notFoundHandler)
      .on('modify', modifyHandler)
      .on('modify/:name', modifyHandler)
      .on('/', defaultHandler);
      router.resolve('/v1/a/b/c');
      router.resolve('/v1/modify');
      router.resolve('/v1/modify/test');

      expect(defaultHandler).to.not.be.called;
      expect(notFoundHandler).to.be.called;
      expect(modifyHandler).to.be.calledTwice;

@ifig
Copy link
Author

ifig commented Nov 3, 2016

It works like a charm, thank you!

@ifig
Copy link
Author

ifig commented Nov 3, 2016

I might have spoken too fast. I never enter the defaultHandler ('/').

@krasimir
Copy link
Owner

krasimir commented Nov 7, 2016

@ifig sorry, can you please post your route definitions and the path that you are running against them. I can't reproduce the bug.

@krasimir krasimir reopened this Nov 7, 2016
@richardsimko
Copy link

I have a similar issue where I have to specify the root URL as an absolute URL, otherwise it won't replace it correctly. I'm trying to achieve an app where every 404 redirects to the root (/) but the root handler won't work.

const router = new Navigo(null, false);
router.notFound(function() {
  console.log('404');
  window.location = '/';
});

router.on('/route1', function() {
  console.log('route 1');
});

router.on(function() {
  console.log('root')
});

I've tried initializing the router with new Navigo(null, false), new Navigo('/', false), new Navigo('', false) and none of them work, the root is never called. However new Navigo('http://localhost:8080', false) works but obviously only when running it locally.

I've also tried to use a base path (For example /test but that didn't help).

@krasimir
Copy link
Owner

Thanks for the report @richardsimko I'll dig into that soon.

@richardsimko
Copy link

Nice, thanks!

@sergiopvilar
Copy link
Contributor

@richardsimko solved by using:

new Navigo(location.protocol + "//" + location.host, false)

@richardsimko
Copy link

Cool, thanks!

@krasimir krasimir closed this as completed Feb 5, 2017
@finppp
Copy link
Contributor

finppp commented Mar 21, 2017

Is this not more of a workaround? I would expect the default root path to still work with .notFound and a main/home without using :
new Navigo(location.protocol + "//" + location.host, false)

Thanks

@krasimir
Copy link
Owner

@finppp can you please post your route definitions and give us an example of what's broken. I find difficult to write a test case that replicates your problem.

@finppp
Copy link
Contributor

finppp commented Mar 22, 2017

router = new Navigo(null,false,'#!');
router
.notFound(function(query) {console.log("404");navHome()})
.on({
    '/': function () { navHome() },
    '/my-posts': function () { navMyPosts() },
  })
  .resolve();

This routes to home when I go to localhost:5000/asdf
Same result if I switch the line after .on({ to:
'': function () { navHome() },

I apologise if I have overlooked something

Also, it does work with
router = new Navigo(location.protocol + "//" + location.host,false,'#!');

Thanks

krasimir pushed a commit that referenced this issue Mar 25, 2017
@krasimir
Copy link
Owner

krasimir commented Mar 25, 2017

@finppp I added a test case and it seems that I can't reproduce the bug. Is Navigo operating at the very top folder of your site. I mean is it like under site.com/... or it is somewhere deeper like site.com/web/app/....

Here's the test which had to reproduce the bug:

describe('and the problem described in issue #56-2', function () {
  it('should fire the notFound handler', function () {
    var router = new Navigo(null, false, '#!');
    var notFoundHandler = sinon.spy();
    var homeHandler = sinon.spy();
    var postsHandler = sinon.spy();

    router
      .notFound(notFoundHandler)
      .on({
        '/': homeHandler,
        '/my-posts': postsHandler
      });

    router.resolve();
    router.resolve('asdf');

    expect(homeHandler).to.be.calledOnce;
    expect(notFoundHandler).to.be.calledOnce;
  });
});

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