visionmedia / page.js

Micro client-side router inspired by the Express router
http://visionmedia.github.com/page.js
7.67k stars 687 forks source link

add routerId to config options: support multiple instances of page #493

Closed kyle-west closed 3 years ago

kyle-west commented 5 years ago

When an application has multiple heroku apps mounted under the same domain and each app uses PageJS as its client-side router, the browser back/forward buttons don't work between the seperate apps. This is because the popstate event handler of each instance assumes that it is the only entity modifing the history state, and therefore tries to route paths that it may not own.

For this reason, I added the option for a routerId to disinguish between multiple apps (that each use a different instance of page). Each instance now includes its id in the history state, and that id is used by page to know whether to route on the client's side, or defer to the server.

coveralls commented 5 years ago

Coverage Status

Coverage decreased (-0.4%) to 88.95% when pulling f03206a92796c06809e30d9914246fa538fd7343 on kyle-west:multiple-instances-of-page-in-same-domain-support into 71bb2d8771159985065819fcebd8e6b5c62e3b26 on visionmedia:master.

dthorne commented 5 years ago

This would be great.

matthewp commented 5 years ago

Multiple page instances is now available in 1.10.0 by calling page.create(). Thanks though!

kyle-west commented 5 years ago

@matthewp I should have been more explicit that this pull request is for a different use case than page.create(). The create function allows for multiple instances of page to live inside the same app. My change allows for navigation between separate apps (all living under the same domain) that each use page routing. I will give an example below to be more clear as to why.

Say you have a website called http://website.com. website.com uses load balancing to tie multiple heroku apps under the same domain. One heroku app is mounted at /foo another at /bar and the last at /baz. In the current state of PageJS, when the user navigates from /foo to any of the other two apps and then tries to use the back button to return to the previous app, the URL is updated, but no navigation occurs. This is because the popstate event handler of each instance assumes that it is the only entity modifing the history state, and therefore tries to route paths that it may not own.

My pull request solves this issue by using an instance id stored in the history state to know who owned each route stored in history.

drewswaycool commented 5 years ago

Yes, this would still be nice.

matthewp commented 5 years ago

Ok, sorry about that. I'll reopen. I'm going to mull over the issue in my head some and get back to you.

kyle-west commented 5 years ago

Thank you. I noticed there are significant merge conflicts with my branch now. I'd love to resolve them after you have had a chance to think about it.

matthewp commented 5 years ago

Ok, I think I'm understanding this more. So let's say the user goes directly to example.com/bar. And then goes to /foo. Because /foo is not part of this app's routes the browser does a full navigation to /foo. At this point I would expect the history state to not exist. So if the use hits back I would expect e.state to be undefined. Am I misunderstanding the scenario?

kyle-west commented 5 years ago

You are close. From all my debugging, I have observed that e.state is defined even from a full redirect because both apps use page, which modifies the history state when navigating. Modern browsers at least do not discard the state when a full navigation occurs.

matthewp commented 5 years ago

Interesting... I'm going to test this out for myself if you don't mind. So are you solving this by assigning different ids in your app? I would rather avoid putting that burden on users. Can we just use Math.random() instead? Would presumably mean either page would be unique....

matthewp commented 5 years ago

I tried to recreate this here: https://gist.github.com/matthewp/6afe8de556e7a0db0ae8ea25db2e0930

I start out on page1.html and navigate to page2.html. From there I hit the back button. The popstate event does not fire, it takes me to page1.html.

What am I doing wrong?

kyle-west commented 5 years ago

First off, thanks for working with me to try and understand this. I am sure we will be able to get on the same page soon.

Second, I like the idea of using Math.random(), but for debugging purposes, it would be better to use the random function only as the default id if the consuming app does not supply one.

Third, I will admit that recreating my problem locally is a pain. It only happens in a very specific case and is extremely hard to debug. There are a number of steps:

  1. create an example app similar to your given example as a node instance running on one port
  2. make another node instance running on a separate port with different routes than the first app (we will call it page3 and page4 in this example)
  3. run a third node server that proxies the first app at /foo and proxies the second app at /bar (to simulate a load balancer)

I am not certain if all of that is necessary, but it is the way to recreate my situation as closely as practical. The following are the steps to recreating the problem once you have set things up.

  1. open the third app in your browser and navigate to /foo/page1.html
  2. go to the about page, then to page 1, then to the about page again
  3. navigate to /bar/page3.html
  4. navigate to the page4, then to page 3 again.
  5. push the back button to traverse through your history
  6. Note that after you go back from /bar to /foo routes, that the url will update when the back button is pressed, but the page will not update.

I'm so sorry this is such a hard problem to recreate. I have been working on this bug for sometime. I have a fork that we have been using in our production environment for weeks, and this solution seems to have solved our routing problems. I submitted this PR to try and help others that might run into this problem too. Thanks for working with me on this.

kyle-west commented 3 years ago

This has been open for over 2 years. I see it will probably never get merged. I guess the fork I made all those years ago for my team's codebase will have to do (since it seems to have been working for us all this time).

Thanks for your time! I appreciate how patient you were with trying to understand these changes.