Navigo is not working properly from version 8.8.5

[stepanjakl]

Hi,

For some reason Navigo is working only partially from version 8.8.5. Everything works fine with v.8.8.4.

I have a pretty standard setup:

window.router = new Navigo('/')

window.router.hooks({
	before: function (done, match) {
...
})

window.router.on({
	'*': function ({ data }) {
...
})

Only the before hook gets fired, but other hooks, routes and links are not working as expected.

I am using the standard UMD (global variable) file to run the Navigo lib.

Any ideas?

[krasimir]

Hey @stepanjakl,

can you try with 8.8.6. I accidentally pushed 8.8.5 without some changes.

[stepanjakl]

@krasimir Same with 8.8.6, I forgot to mention

[krasimir]

Hm ... ok. Thanks. I'll investigate.

[krasimir]

Hey @stepanjakl,

can you provide a more complete test case. I wrote a unit test exercising the generic hooks with a route * and everything seems fine:

const r: NavigoRouter = new Navigo("/");
const before = jest.fn().mockImplementation((done) => done());
const after = jest.fn();
const leave = jest.fn().mockImplementation((done) => done());
const already = jest.fn();
const handler1 = jest.fn();
const handler2 = jest.fn();

r.hooks({
  before,
  after,
  leave,
  already,
});
r.on("/books", handler2);
r.on("*", handler1);

r.navigate("/foo/bar");
r.navigate("/foo/bar");
r.navigate("/bar/foo");
r.navigate("/books");

function expectCall(mock, numOfCall, numOfArg, url) {
  expect(mock.mock.calls[numOfCall][numOfArg]).toEqual(
    expect.objectContaining({ url })
  );
}

expect(handler1).toBeCalledTimes(2);
expectCall(handler1, 0, 0, "foo/bar");
expectCall(handler1, 1, 0, "bar/foo");

expect(before).toBeCalledTimes(3);
expectCall(before, 0, 1, "foo/bar");
expectCall(before, 1, 1, "bar/foo");
expectCall(before, 2, 1, "books");

expect(after).toBeCalledTimes(3);
expectCall(before, 0, 1, "foo/bar");
expectCall(before, 1, 1, "bar/foo");
expectCall(before, 2, 1, "books");

expect(already).toBeCalledTimes(1);
expectCall(already, 0, 0, "foo/bar");

expect(leave).toBeCalledTimes(1);
expectCall(leave, 0, 1, "books");

expect(handler2).toBeCalledTimes(1);
expectCall(handler2, 0, 0, "books");

Also the other unit tests are passing. So I'm struggling to reproduce on my end.

[stepanjakl]

Thanks for that!

I figured out what is causing the router to stop.

In the before hook, I have a condition to re-route in a case the match data is empty:

if (!match.data) {
    window.router.navigate('en', { callHandler: false })
}

The script enters the condition, but then it stops, without any error or warning. If the condition is voided, then the router works fine. I hope it is okay to have the navigate function inside the hook. Nevertheless, it worked in the v.8.8.4.

I am expecting the re-route to be caught here:

window.router.on({
	':language': function ({ data }) {
        ... 
     }
  })

[krasimir]

@stepanjakl can you please try the new 8.8.7 version. Indeed there was a problem with the before hook (also exists on the leave hook). Let me explain a bit what was the problem since you probably have to add one small thing to your code. While the router is resolving a route it sets itself into a "dirty" mode. This guarantees that no other actions will happen until the work for the current route is done. The before and leave hooks have this option to stop the flow and the bug was that the router stayed into that "dirty" mode. In 8.8.7 I'm introducing a fix which makes sure that we go out of this state. The thing is that you have to inform explicitly Navigo that you are blocking the execution. So, your before hook should looke like that:

before: (done, match) {
  if (!match.data) {
    done(false); // <-- this will make dirty=false and will allow the subsequent navigation
    window.router.navigate('en', { callHandler: false })
  }
}

[stepanjakl]

@krasimir Yes, it works fine now!

The 'dirty' mode makes sense in some use cases, but I would suggest to put it in the function's call as a parameter (if that's possible). Like this:

before: (done, match, dirty = false) {
  if (!match.data) {
    window.router.navigate('en', { callHandler: false })
  }
}

What do you think?

And add some explanation about this to the documentation.

Again, thank you for the quick help and solution :)

[krasimir]

I'll push back a bit on your suggestion. The dirtyness of the router is really an internal implementation detail. It shouldn't be exposed to the consumer of the library.

[krasimir]

Also I should thank you for rising the issue. That's the only way to make Navigo robust/stable.