navi icon indicating copy to clipboard operation
navi copied to clipboard

`basename` issues

Open pcvonz opened this issue 5 years ago • 7 comments

I can't seem to get the basename property on navigation to work. I assume it serves the exact same purpose as react router's basename.

Redirects and links seem to ignore it. Here is my main <App /> component:


function App() {
    const dispatch = useDispatch();
    const { profile } = useReduxState('user');
    const { fieldTypes } = useReduxState('app');

    useEffect(() => {
        dispatch(gapiInit());
    }, [dispatch]);

    return (
        <Router
            routes={routes}
            basename={BASEPATH}
            context={{ profile, fieldTypes }}
        >
            <MainLayout>
                <Suspense fallback={null}>
                    <View />
                </Suspense>
            </MainLayout>
            <Message />
            <ConfirmationDialogue />
        </Router>
    );
}

My main routes file


const routes = mount({
    '*': map((request, context) =>
        !context.profile || !context.fieldTypes
            ? redirect(
                  '/login?redirectTo=' +
                      encodeURIComponent(request.originalUrl),
                  { exact: false }
              )
            : map(() => {
                  store.dispatch(setConfirmRouteChange(false));
                  store.dispatch(setConfirmationDialogue(null));
                  return pageRoutes;
              })
    ),
    // Catches the root URL before login to avoid adding 'redirectTo' by default
    '/': map((request, context) =>
        !context.profile || !context.fieldTypes
            ? redirect('/login')
            : pageRoutes
    ),
    '/login': map(async (request, context) =>
        !context.profile || !context.fieldTypes
            ? route({
                  title: 'Login',
                  view: <LoginPage />
              })
            : redirect(
                  request.params.redirectTo
                ? decodeURIComponent(request.params.redirectTo)
                      : '/'
              )
    )
});

BASEPATH is set to /dist/. Navigating to localhost:8080/dist redirects me to localhost:8080/login.

Any help would be greatly appreciated :pray:

pcvonz avatar Jun 20 '19 22:06 pcvonz

The basename option is a little different in Navi to react-router -- it does affect routing, but doesn't effect links/redirects. This allows basename to be used to integrate Navi with react-router and other routers, but also means that you'll need to pass the full path for links/redirects.

One way to do this would be to add basename to the routing context and add it into redirects manually.

jamesknelson avatar Jun 21 '19 02:06 jamesknelson

Hmm, adding the basename to context is a solution for routing, but I also have to add it to every call to navigation.navigate. After playing around with it for a little longer I found something that works well for my use case. If there was a nice way to hook into navigation.navigate and modify or cancel the call that'd be awesome. Currently I havenavigation.navigate monkey patched (:speak_no_evil: :face_with_head_bandage:), which is not elegant, but it works. It also adds the ability to stop a navigation request based on certain criteria (if a form is partially filled out for example).

pcvonz avatar Jun 21 '19 22:06 pcvonz

There's also the option of using the navigation blocking features of the underlying history object. To do this, you can create a history manually and pass it in. This is how I've been doing it so far, but if you've come up with your own API I'd love to see it though, as it might come in when I eventually remove the dependency on history.

jamesknelson avatar Jun 22 '19 09:06 jamesknelson

Here is what I came up with, fairly simple change. It needs to be fleshed out a little more, but this is the basic idea: https://github.com/bargreenellingson/navi/commit/419082db1d0a4028e2277709c7fd93c842c46d75

Here is an example function that could be passed into preNavigation:

function preNavigation(nextURL) {
    if (
        // Don't add public_path if it's already added
        !nextURL.pathname.includes(PUBLIC_URL) &&
        // Ensure relative urls still work
        nextURL.pathname[0] === '/'
    ) {
        nextURL.pathname = PUBLIC_URL + nextURL.pathname;
    }
    const confirmRouteChange = store.getState().app.confirmRouteChange;
    if (confirmRouteChange) {
        store.dispatch(
            actions.setConfirmationDialogue(
                confirmRouteMessage(navigation, nextURL)
            )
        );
        // Ignore this navigation request
        return null;
    }
    return nextURL;
}


pcvonz avatar Jul 01 '19 21:07 pcvonz

This is definitely something we need to add to the API at some point. Perhaps we could call if onBeforeNavigate?

The issue I see with this is that doesn't allow asynchronous confirmation of changing route, and it doesn't allow blocking navigation when navigate() isn't used. I think this might need a little more thought.

jamesknelson avatar Jul 10 '19 14:07 jamesknelson

Hi.

I'm trying to get my app working from a subdir. But after playing for a couple of hours with CRA configuration and Navi Router basename I incline to the point that it's more not possible than possible, or at least not reasonable than reasonable.

Since just to get this working I need to rewrite my whole app, substituting new base path (which is actually the PUBLIC_URL in the context of CRA) peripodically:

  • in links
  • in redirects
  • in navigation.navigate

In contrast, with react-router I can just pass the basename once — and it works. I haven't checked its redirects but I suspect it's gonna be fine as well.

So there is basically no a robust way to provide the basename in one place and get it working when using Navi. And if I understood it correctly, this inability we owe to the compatibility with react-router. Couldn't there be just one parameter to the Router saying: "No, I don't need react-router, I use Navi because I have a lot o async stuff, so please just don't make my brain and use this basename for everything", eh?

Meanwhile I'm gonna stop any further researches and report back to our devops that we cannot get our app working in subdirs e.g. "/develop", "/master" etc :(

OnkelTem avatar Jun 25 '20 12:06 OnkelTem

Hmm, adding the basename to context is a solution for routing

Not in our case I'm afraid. We have dozens of components and, with a minor exception, they're all written with the router-agnostic thought in mind. To give them Navi context woud mean editing all of them and making them more navi-dependent either by calling useCurrentRoute() from inside of the components or by wrapping all the components into a custom withRouter() hoc to get the context basename passed in props.

OnkelTem avatar Jun 25 '20 12:06 OnkelTem