danielr18 / connected-next-router

A Redux binding for Next.js Router compatible with Next.js.
MIT License
109 stars 30 forks source link

Doesn't consider basePath? #78

Open laggingreflex opened 2 years ago

laggingreflex commented 2 years ago

Does this not take basePath into consideration?

I have "/frontend" as my basePath and I'm seeing this in the logs (using redux-logger):

CHANGED: router.location.href /frontend/auth/login → /auth/login
CHANGED: router.location.pathname /frontend/auth/login → /auth/login

Not sure what the expected behaviour should be, but:

Great module BTW!

laggingreflex commented 2 years ago

I can't reproduce it in the examples given, but it's definitely causing weird artifacts in my app. Like redundantly duplicating basePath "/frontend/frontend/...", and subsequently redirecting to it which obviously results in 404. I can't yet figure out why or where this is happening, or whether its my own app or this library...

But I've found a workaround that seems to avoid the issue of unnecessary redirection and path changes.

By patching next/router:

import Router from 'next/router'
import { createRouterMiddleware, ConnectedRouter } from 'connected-next-router';

/* Make `Router.asPath` return with `basePath` prefixed (if absent) */
const patchedRouter = new Proxy(Router, {
  get: (Router, key, receiver) => {
    if (key === 'asPath') {
      const { basePath, asPath } = Router
      const replaced = asPath.includes(basePath) ? asPath : basePath + asPath
      return replaced
    } else {
      return Reflect.get(Router, key, receiver)
    }
  }
})

createRouterMiddleware({ Router: patchedRouter })

<ConnectedRouter Router={patchedRouter}/>
danielr18 commented 2 years ago

Hi @laggingreflex, I'm currently sick and can't investigate the issue at the moment. I should be able to next week.

polaroidkidd commented 2 years ago

I'm also seeing this behavior, but only when using query parameters. Otherwise great work!