kadirahq / flow-router

Carefully Designed Client Side Router for Meteor
MIT License
1.09k stars 192 forks source link

TriggersExit not firing again after "stop()"ped #491

Open ghost opened 8 years ago

ghost commented 8 years ago

the titles sums it up - I'm unable to have my triggersExit function fire again, after it has been stopped.

this is my code

FlowRouter.route('/lobby/:id', {
    name:'lobby',
    action() {
        BlazeLayout.render('default_layout', {content:'lobby'})
    },
    triggersExit:[(context, redirect, stop) => {
        if(context.params.id
        && Lobbies.findOne({_id:context.params.id, state:{$ne:2}}))
            if(!confirm(`The lobby isn't over.\n\n Are you sure you want to leave this page?`))
                stop()
    }]
})

The trigger is fired the first time I try to leave the route, but won't trigger again before I navigate away from the route or refresh.

I'm not sure if this is a bug, or if I'm approaching this the wrong way :)

ghost commented 8 years ago

I had a previous solution before running into this problem:

Lobby.onCreated(function() {
...
    FlowRouter.triggers.exit([(context, redirect, stop) => {
        if(this.lobbyId && Lobbies.findOne({_id:this.lobbyId, state:{$ne:2}}))
            if(!confirm(`The veto isn't over.\n\nAre you sure you want to leave this page?`))
        stop()
    }], {only:['lobby']})
...
})

however, this solution threw an exception because stop was undefined sometimes... Which is why I refactored in the first place.

arunoda commented 8 years ago

triggersExit only runs once before you exit your page. It's not reactive.

ghost commented 8 years ago

but I didn't actually exit, since I called stop()...

I am assuming this is a bug, and that triggersExit would actually be called the second time as well? :)

ghost commented 8 years ago

I hope this can either be accepted as a bug or dismissed as a misunderstanding on my part :)

I find it quite logical that triggersExit should run a 2nd time, if I stop() it the first time, and don't navigate away

convexset commented 8 years ago

@arunoda: so the problem is that after stop is called, flow-router thinks that we are on the new route. This aspect is not properly handled by the commit which introduces stop: https://github.com/kadirahq/flow-router/commit/cc8b8811a2f44f65c3c91dee839d40b5fe7ac67c

convexset commented 8 years ago

So I have a partial fix which is rather hackish and leaves repeated history entries. I can make a PR but. @arunoda will probably need to fix it up given my experience with flow-router's source is just a few hours.

convexset commented 8 years ago

Fixed relatively comprehensively here: https://github.com/kadirahq/flow-router/pull/681