search

solidjs / solid-router

A universal router for Solid inspired by Ember and React Router
MIT License
1.13k stars 143 forks source link

useBeforeLeave not firing on history.back() #277

Closed KamehamehaNudel closed 7 months ago

KamehamehaNudel commented 1 year ago

Describe the bug

The "useBeforeLeave" hook seems to only be firing when navigating using links. When using the browsers back/forth buttons it does not appear to execute.

Your Example Website or App

https://stackblitz.com/edit/solidjs-templates-ejhtft?file=src%2FApp.jsx

Steps to Reproduce the Bug or Issue

  1. Go to the "test" route using the nav link.
  2. Go back to the "home" routeusing the nav link.
  3. An alert "here" will show up
  4. Go back to the "test" route using the nav link.
  5. Click the "back" button (fires history.back())
  6. The route changes correctly, but the alert does not show up

Expected behavior

As a user i expeted the "useBeforeLeave" function to also fire when using the "back" button (either progromatically or with the actual browser buttons in a 'real' environment). I am in both cases leaving the current route so I don't see why it shouln't execute.

Screenshots or Videos

No response

Platform

Additional context

No response

Brendan-csel commented 1 year ago

The PR that added useBeforeLeave only supports router-initiated blocking. Not browser-initiated blocking such as back/forward buttons.

I do have a fork of this router were I had a go at adding those other blockers. We have been using that in production and it seems to be working fine - but hasn't had broader testing. Also we us the Hash Integration so "normal" integration hasn't been tested. I think this is the commit that adds that ability.

KamehamehaNudel commented 1 year ago

Aye i See. Thanks for the quick answer, I'll see if i can get that running as a custom integration or patch. Will that also make it to main version (eventually) ? Or is it 'intentional' afterall?

Brendan-csel commented 1 year ago

I just happened update that branch of mine recently so if you want to quickly test with it you can try install csel-solid-router from npm and find/replace to update any files that import the router.

Will that also make it to main version (eventually) ? Or is it 'intentional' afterall?

That'll be up to @ryansolid. It is quite a bit of code - although it "works for me".

CreativeTechGuy commented 1 year ago

I just want to chime in that I found this issue because this is something that I was very surprised was missing. Hopefully it can get added!

ryansolid commented 12 months ago

@Brendan-csel I'm open to looking at getting this in. If you have a PR I'd review it.

ryansolid commented 7 months ago

Closed by #363