vercel / next.js

The React Framework
https://nextjs.org
MIT License
127.08k stars 27.01k forks source link

/basePath redirects to root in NextJS 13 (appDir) #41824

Closed wiredearp closed 1 year ago

wiredearp commented 2 years ago

Verify canary release

Provide environment information

Operating System: Platform: darwin Arch: arm64 Version: Darwin Kernel Version 21.6.0: Mon Aug 22 20:19:52 PDT 2022; root:xnu-8020.140.49~2/RELEASE_ARM64_T6000 Binaries: Node: 16.14.0 npm: 8.3.1 Yarn: 1.22.18 pnpm: N/A Relevant packages: next: 13.0.0 eslint-config-next: 13.0.0 react: 18.2.0 react-dom: 18.2.0

What browser are you using? (if relevant)

Firefox 106.0.1

How are you deploying your application? (if relevant)

No response

Describe the Bug

In NextJS 13, the document at /myapp instantly redirects to the root path /.

const nextConfig = {
  basePath: '/myapp',
  experimental: { appDir: true },
  reactStrictMode: true,
  swcMinify: true,
}

During this redirect, or replaceState, the /myapp entry is not registered in history.

While the root app/layout.tsx and app/page.tsx is now rendered on screen, the website becomes a 404 if we reload the location / without the /myapp segment.

It would appear that basePath works differently or not at all in version 13 👀

Expected Behavior

The /{basePath} should stick around in the location so that we can reload forever.

Link to reproduction

https://github.com/wiredearp/basepath-redirect

To Reproduce

Create a website with create-next-app, set basePath and enable appDir. Then run dev and visit the /{basePath} to witness the redirect. This setup can be cloned from the reproduction repo. Thanks for digging in!

cachho commented 2 years ago

I can confirm, this is also stopping me from deploying my nextjs13 project.

It also breaks next/image, but I found the following workaround for that in another thread.

images: {
    path: '/<basepath>/_next/image',
  }, 
alexdgourlay commented 1 year ago

Can confirm also.

The workaround we've come up with makes use of the redirects functionality in the config.

module.exports = {
  basePath: '/myapp',
  async redirects() {
    return ([
      {
        source: '/',
        destination: '/myapp',
        permanent: true,
        basePath: false,
      },
    ]);
  }
};

Of course this is increases loading time with the added redirect.

nenorrellHound commented 1 year ago

Can also confirm this is still happening for me. Sadly the workaround above didn't solve my case, as the path still is not rewritten to the base path on redirect.

I'll add that you can see that basePath DOES work for a fraction of a second before the url path is rewritten back to root.

ozd3v commented 1 year ago

still happening

cachho commented 1 year ago

I deployed my project to production with this issue, here's what I've learned. It covers a case where you have another page at root, and how to deal with it in nginx.

Since I have multiple subpages, including dynamic routes directly under root I had to add a whole set of rules following @alexdgourlay's instructions. Keep in mind that you can use regex, in my case this was required to separate the dynamic routes. This worked in dev, however in prod I had to give up on this idea and instead created a new subfolder above the dynamic route, so it doesn't site directly under root.

/classic
    /[agent]
/new
    /[country]
        /[agent]
            /[line]

Here's what it looks like in my example, my basepath would be /my-basepath:

async redirects() {
    return [
      {
        source: '/new/:country([A-Z][a-z.%20{%20&}]+)',
        destination: '/my-basepath/new/:country',
        permanent: true,
        basePath: false,
      },
      {
        source: '/new/:country([A-Z][a-z.%20{%20&}]+)/:agent/',
        destination: '/my-basepath/new/:country/:agent',
        permanent: true,
        basePath: false,
      },
      {
        source: '/new/:country([A-Z][a-z.%20{%20&}]+)/:agent/:line',
        destination: '/my-basepath/new/:country/:agent/:line',
        permanent: true,
        basePath: false,
      },
      {
        source: '/',
        destination: '/my-basepath',
        permanent: true,
        basePath: false,
      },
      {
        source: '/classic',
        destination: '/my-basepath/classic',
        permanent: true,
        basePath: false,
      },
      {
        source: '/classic/:agent',
        destination: '/my-basepath/classic/:agent',
        permanent: true,
        basePath: false,
      },
    ];
  },

However, in production I have another site at root (the reason I'm using a base path), and unlike dev, (this is my understanding), because the browser goes to root for a split second before the redirect can kick in, my nginx for the site at root is in control and the redirect can't resolve.

The solution for this was to add more rules for nginx. These need to be uniquely identifiable, so that nginx knows they came from the nextjs app and can send you back there.

location /my-route/ {  
        return 301 $scheme://my-site.com/my-basepath/$request_uri;
    }

At this point I decided to give up on /basepath/[country], because I would have to figure out how to rewrite root/[country] in nginx, without redirect any of my main site at root's subpages. So I moved [country] into /new, in my example I just have to redirect /new and /classic in nginx. One of the problems caused by this is that my site at root can't have /my-route, because that would be redirected to your nextjs-app.

The next problem is that you can't link back to / in your nextjs-app. This will direct you to the root of your page at route, and I can't map a redirect for that. My solution was to add these two redirects:

      {
        source: '/my-basepath/home',
        destination: '/my-basepath',
        permanent: true,
        basePath: false,
      },
      {
        source: '/my-basepath/my-basepath/home',
        destination: '/my-basepath',
        permanent: true,
        basePath: false,
      },

everywhere I have a link to / in my next-app, I replaced it with /my-basepath/home. So yes, this goes on top of the basepath, so the link on the web looks like this /my-basepath/my-basepath/home:

<Link href="/my-basepath/home">
    <Image src={Logo} alt="website logo"></Image>
</Link>

If use the broken redirect from within the app, the base path is swallowed as we know, and it goes to /my-basepath/home (in my case /my-basepath/home), which is where the first of the two added redirects kicks in. If someone opens in a new tab, the second rule kicks in. Reminder, I'm doing this because /basepath redirects to root, and I can't redirect the root of my domain. This leaves me with an ugly home link.

Because I handle it through nginx now, all of the nextjs/next.config.js redirects except those for home could be removed. However, this won't work in dev, so I'll personally be keeping them.

Bottom line, things get worse in production when you actually have something at root. You need lots of redirects which makes links ugly and the performance slow. And if you change something you have to look in lots of places. I'm sure there are better ways to do this, and that nginx has advanced functions to deal with this.

I know it's Christmas and everyone deserves their time off. Images with basepath were fixed, and I hope routes/links are next. Thanks devs!

rzseattle commented 1 year ago

This bug holding back my app upgrade. :/

nmiddendorff commented 1 year ago

This issue went away after my upgrade to 13.1.1. 🎉

michael-land commented 1 year ago

It still not working properly on 13.1.2 canary.

// next.config.js
module.exports = {
    basePath: '/docs',
    experimental: { appDir: true },
}
export default function HomePage() {
  return (
      <Link href="/style-guide">Style Guide</Link>
  )
}

when hovering, the browser shows the correct url localhost:3000/doc/style-guide, but it navigate to localhost:3000/style-guide.

According to the documentation, the expected url should be /doc/style-guide

For example, using /about will automatically become /docs/about when basePath is set to /docs.

alexdgourlay commented 1 year ago

Resolved for us upon updating to v13.1.1 😄

cachho commented 1 year ago

I created issue #44893 to add a codesandbox example for the latest canary release. I don't know how you guys solved it, it's still happening for me (and in the code sandbox).

Matrix89 commented 1 year ago

I'm also having this issue, while looking at the source it seems to me like the base path should also be added here, here, and here the same way as here if somebody from the next.js team can confirm that I'd like to create a PR

EDIT: I've also dug deeper thinking maybe the router should be responsible for adding the base path but I don't see anything

Nevermind, I'm still having the issue but it's router related?

RonHouben commented 1 year ago

Also in NextJS 13.1.5 the issue still seems to exist.

kjpluck commented 1 year ago

Have successfully resolved this. As is usual it's all about slashes!

Make sure your basepath in next.config.js begins with a slash and doesn't have one on the end:

const nextConfig = {
  reactStrictMode: true,
  basePath: '/folder_name'
};

module.exports = nextConfig;

and your nginx config does NOT end in a slash:

       location /folder_name    # NO SLASH ON END!
       {
                proxy_pass http://localhost:3000/folder_name;     #NO SLASH ON END!
       }
cachho commented 1 year ago

Have successfully resolved this. As is usual it's all about slashes!

Make sure your basepath in next.config.js begins with a slash and doesn't have one on the end:

const nextConfig = {
  reactStrictMode: true,
  basePath: '/folder_name'
};

module.exports = nextConfig;

and your nginx config does NOT end in a slash:

       location /folder_name    # NO SLASH ON END!
       {
                proxy_pass http://localhost:3000/folder_name;     #NO SLASH ON END!
       }

it's nice when you can use redirects, but if you have to use a hosting solution where you're deploying under a basepath, and you have no permission to redirect (especially from the root), then this doesn't work. That's why it's just a work around and nextjs should have a selfcontained solution.

basarane commented 1 year ago

I can confirm that the issue is resolved in v13.1.7-canary.9. Didn't test canary.10 yet.

github-actions[bot] commented 1 year ago

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.