hyperapp-router icon indicating copy to clipboard operation
hyperapp-router copied to clipboard

Router is getting confused when using baseUrl.

Open diasbruno opened this issue 3 years ago • 6 comments

Using baseUrl, the router still requires that the navigation link, and when declaring a route, to have the baseUrl.

Steps to reproduce

Simple configuration...

const router = {
  baseUrl: '/test',
  
  RouteAction: (state, { path: url }) => ({ ...state, url }),

  routes: {
       '/': () => div({}, text("home")),
       '/a': () = div({}, text("a"))
       '/test/a': () = div({}, text("a")) // <- also when declaring a route.
  }
};
navigation('/a') // doesn't match any route
navigation('/test/a') // match

Expected

Declaring a route and navigation('/a') should not require the baseUrl.

Reproducible example

diasbruno avatar Mar 10 '21 22:03 diasbruno

Just to make sure this is the correct behavior...

If I install the app in a folder '/path', all the links must resolve 
using the `baseUrl`, example: '/a' must resolve to '/path/a'.
And if I want to go a path below, I must use the full URL.

diasbruno avatar Mar 11 '21 12:03 diasbruno

Man, I'm sorry I dropped the ball on this. Without causing a lot of global state or side effects, I think including the baseUrl path with the navigate calls makes the most sense. I will look into this a bit more, though, because I agree that it doesn't feel intuitive.

mrozbarry avatar May 26 '21 17:05 mrozbarry

Nah...no problem.

I think including the baseUrl path with the navigate calls makes the most sense.

You mean internally navigate will deal with it, or, the users will need to pass the entire path to the navigate ?

diasbruno avatar May 26 '21 17:05 diasbruno

For the current iteration of the code, I think your app will have to specify the full path. So navigate('/your/base/path/stuff') for now, but I am going to think this through a bit more, I don't know that I'm satisfied with that, and I agree that you original report and expectations should be the correct version.

mrozbarry avatar May 26 '21 18:05 mrozbarry

So I do have a thought, tell me if this fantasy and not-implemented version of the API would work better for you:

router.js:

import { container } from 'hyperapp-router';

export default container({
  // ... your routes
}, {
  baseUrl: 'foo/bar',
  hookOnAnchorClicks: true,
});

index.js:

import { app, h, text } from 'hyperapp';
import router from './router.js';

const handleButtonAction = (state, event) => {
  event.preventDefault();
  return [
    state,
    router.navigate('baz'), // Navigate to '/foo/bar/baz'
  ];
};

router.hof(app)({
  // your app definition here, like usual
  // no need for the extra router config, that lives in your router now

  render: (state) => {
    return h('div', {}, [
      h('button', { onclick: handleButtonAction }, text('This button navigates')),
      h('a', { href: '/foo/bar/baz' }, text('This anchor will use the hyperapp-router, because the route exists')),
  },
});

This will mean that your router will have a singleton instance, but that will make the router.navigate have the right context. It also means you can share it around a bit, too, but I'd recommend putting the router in your state so it doesn't cause a side-effects in your components.

mrozbarry avatar May 26 '21 20:05 mrozbarry

@mrozbarry I'm not sure if my comment belongs to this issue or I should create a separate one. Please, do let me know if I should move it over.

There are a few features that would be great to see on hyperapp-router:

  1. To declare the components for each route alongside their hooks.
  2. Within the component, have access to the existing routes available in the app. This is handy in case of refactoring of URLs.
  3. Within the component, be able to identify if the user is currently in a particular route. Useful in case it's a (sub)component being rendered in multiple routes.
// account.page.js
export const AccountPage = (state) => html`<div>Account</div>`

// account.hooks.ts
export const AccountHooks = {
  onEnter: (state) => [state, [fx, {}]],
  onLeave: (state) => [state, [fx, {}]]
}

// routes.js
export const routes = {
  account: route('/account/:userId', () => AccountPage, AccountHooks)
}

// state.js
const config = {
  getter: (state) => state.router,
  setter: (state, router) => ({ ...state, router }),
}

export const state = {
  router: Router.init(routes, config)
}

In other parts of the app, I can do something like this:

function OnLogin(state) {
   const redirectTo = routes.account.url({ userId: state.userId })
   return [state, pushUrl(redirectTo)]
}

Is this something that would be valuable for hyperapp-router?

hlibco avatar Jun 16 '21 09:06 hlibco