Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Middleware: Support for basePath in the app directory #243

Closed
amannn opened this issue Apr 14, 2023 Discussed in #242 · 23 comments · Fixed by #699
Closed

Middleware: Support for basePath in the app directory #243

amannn opened this issue Apr 14, 2023 Discussed in #242 · 23 comments · Fixed by #699
Labels
contributions welcome Good for people looking to contribute enhancement New feature or request

Comments

@amannn
Copy link
Owner

amannn commented Apr 14, 2023

Currently the middleware assumes that / is the root of the app and works on top of that.

We could adapt it as follows:

  1. Don't hardcode the root URL
  2. All the places where an absolute URL is constructed, the basePath needs to be considered
    urlObj.pathname = urlObj.pathname.replace(`/${locale}`, '');
  3. Somehow the basePath needs to be passed to the middleware. Maybe we could use the plugin that is worked on in feat!: Next.js 13 RSC integration #149 to read it from the Next.js config and then make it automatically available to the middleware (e.g. with webpack.DefinePlugin).

Potentially this could also help to implement region-specific routing, see #243 (comment) and #549

(discussed in #242)

@amannn amannn added the enhancement New feature or request label Apr 14, 2023
@amannn
Copy link
Owner Author

amannn commented Apr 14, 2023

Hey, thanks for starting this discussion! This is definitely a valid feature request, I'll convert this to an issue.

I just saw that in the pages folder with i18n routing, Next.js appends the locale to the base path (e.g. domain.tld/base-path/en/about and even domain.tld/base/path/en/about). Is that also what you need?

@setowilliam
Copy link

I'm having this problem too. It only happens when I try to go to the locale set to defaultLocale. For example:

export default createMiddleware({
  // A list of all locales that are supported
  locales: ['en', 'ja'],

  // If this locale is matched, pathnames work without a prefix (e.g. `/about`)
  defaultLocale: 'ja'
});

and next.config.js

const nextConfig = {
  reactStrictMode: true,
  basePath: '/path',
  experimental: {
    appDir: true
  }
};

If I try to navigate to /path/en, it will be fine. But navigating to /path/ja or /path will redirect to /404

@paqstd-dev
Copy link

paqstd-dev commented May 15, 2023

Similar problem. The Next 13.4 documentation has basePath which doesn't work for this library.

I use 2.15.0-beta.2 version next-intl and I have bug:

middleware.ts:

import createMiddleware from 'next-intl/middleware'
import { locales, defaultLocale } from 'shared/lib/i18n'

export default createMiddleware({
  // A list of all locales that are supported
  locales: [...locales],

  // If this locale is matched, pathnames work without a prefix (e.g. `/about`)
  defaultLocale,

  // Include a prefix for the default locale as well, you can configure the middleware accordingly.
  localePrefix: 'always',

  // Disable automatic locale detection
  localeDetection: false,
})

export const config = {
  // Skip all paths that should not be internationalized
  matcher: ['/((?!api|_next|.*\\..*).*)'],
}

next.config.js:

const withNextIntl = require('next-intl/plugin')(
  // This is the default (also the `src` folder is supported out of the box)
  'shared/lib/i18n/index.ts'
)

const isProduction = process.env.NODE_ENV === 'production'

/** @type {import('next').NextConfig} */
const nextConfig = {
  assetPrefix: isProduction ? '/path' : '',
  basePath: isProduction ? '/path' : '',
  rewrites() {
    return isProduction
      ? [
          {
            source: '/path/_next/:path*',
            destination: '/_next/:path*',
          },
        ]
      : []
  },
}

module.exports = withNextIntl(nextConfig)

Redirected to http://localhost/en/path but not http://localhost/path/en.

I think this may be help: https://nextjs.org/docs/pages/api-reference/next-config-js/runtime-configuration.

@paulvanbrenk
Copy link

I've been fighting this for the past week. I believe it's a bug in NextJS, but I'm not getting much of a response from them.

vercel/next.js#49883

@dbrxnds
Copy link

dbrxnds commented May 22, 2023

Hi, just chipping in to say that we are currently running into this same issue. Similarly to @setowilliam this only seems to be an issue for the default locale.

@amannn
Copy link
Owner Author

amannn commented May 22, 2023

I haven't looked deeper into this and currently have to look into a few other things. So if someone is interested in contributing this feature, you're more than welcome to!

Middleware: https://github.com/amannn/next-intl/blob/main/packages/next-intl/src/middleware/middleware.tsx
Tests: https://github.com/amannn/next-intl/blob/main/packages/next-intl/test/middleware/middleware.test.tsx

Just leave a note here if someone wants to look into this, so we don't do the work redundantly!

@dbrxnds
Copy link

dbrxnds commented Jun 8, 2023

Hi, maybe I am stating the obvious here but sharing it anyway for those who it may help. Setting the localePrefix property in middleware to always at least makes it possible to work with the default locale. This works out for us as that is our preferred method anyway.

Then in the root page.tsx (above the [locale] folder) you can add an instant redirect('/{my-base-path}/{default-locale}') to make sure it always redirects correctly.

I am aware this isn't really a permanent solution but maybe it helps someone bridge the gap for now.

@amannn amannn changed the title Support for basePath in the app directory Middleware: Support for basePath in the app directory Jul 3, 2023
@fchienvuhoang
Copy link

vote this!

@amannn amannn added contributions welcome Good for people looking to contribute and removed good first issue labels Jul 10, 2023
@koyama-tagbangers
Copy link

I'm interested in improving this feature.
It would be desirable to read basePath option from next.config.js, but how about implement additional basePath option to MiddlewareConfig interface temporarily.
I think that it's a good place to start by improve middleware feature.

@amannn
Copy link
Owner Author

amannn commented Jul 20, 2023

@koyama-tagbangers Great, let me know if you need help!

Starting with an option on the middleware is likely a good idea, yes—at least for the beginning. By doing this, we could already release the feature to the stable channel, since there's no plugin there yet. Potentially we could add an automatic default with the plugin later.

@amannn
Copy link
Owner Author

amannn commented Jul 28, 2023

Side note: I think basePath support on the middleware, especially if it can be configured separately from next.config.js, could potentially help to implement region-specific locales (e.g. example.com/eu/en-GB). /[region] (or /:region, TBD)` needs to be a valid base path then. This could also be added in a second step though, if it increases complexity.

@ivanafanasyeu
Copy link

Hi, any updates here. Is it any way to make it work with basePath?

@J4v4Scr1pt
Copy link

To bad, need to choose another lib because of this.. 😑

@LeopoldLerch
Copy link

we had to switch back to pages-router because of this. it works there

@amannn
Copy link
Owner Author

amannn commented Oct 9, 2023

I think @dellacagna found a workaround for this in #549. It's a bit of an advanced use case to explore though, see e.g. my note about the navigation APIs below.

I'd really like to have built-in support for basePath at some point, but couldn't find the time so far to look into it. Definitely give the top comment a thumbs up if you're interested in this feature so we can prioritize and share your requirements in case they are more complex like the one described in #549.

@FlorianLeChat
Copy link

This should really be written into the docs. I spent two days migrating my website to realize during production that basePath is not yet supported... it's pretty frustrating.

@amannn
Copy link
Owner Author

amannn commented Oct 22, 2023

Sorry to hear about your frustration @FlorianLeChat! basePath is definitely on the radar.

Is your use case a static basePath or something more dynamic like region prefixes (e.g. /europe/fr)? I think the former should be rather straightforward to integrate (see also #243 (comment)). With the new RSC integration, we now have a plugin, which would allow next-intl to read the basePath from the config and to distribute it via webpack.DefinePlugin to the relevant APIs of next-intl (middleware, navigation APIs).

Please let me know if someone wants to step in to work on this! I currently still have a few other priorities, but if no one else looks into it, eventually I'll try to take care of it. I can't promise an ETA at this point though.

@FlorianLeChat
Copy link

Is your use case a static basePath or something more dynamic like region prefixes (e.g. /europe/fr)?

Thanks for your quick reply! Honestly, about 90% of developers in my opinion who use a base path will always use it statically. I'd really like to help you, but unfortunately it's definitely beyond my skills.

@Robjam
Copy link
Contributor

Robjam commented Oct 27, 2023

Hey @amannn I have a fairly rough branch on my fork that I have gotten working locally via yarn link and tests are passing.
I had a couple of questions before creating a PR.

  1. This will probably create some conflicts with the 3.0 branch, is that going to be an issue for you?
  2. Would targeting the 3.0 branch work better? (My team has our first release coming up next month and we would really like to have basePath support)
    Here's the tentative diff
    It is probably easier to progress via PR, so I added some docs.

@AlejandroSanchez90
Copy link

Any eta on this?

@kjxbyz
Copy link

kjxbyz commented Nov 15, 2023

Same issue

@wildskyf
Copy link

I just found a workaround, here is it:
https://blog.wildsky.cc/posts/workaround-for-nextjs-next-intl-basepath#code

Although you may not be able to read Chinese characters, I believe that you only need to read the code to solve the problem. Good luck!

amannn added a commit that referenced this issue Dec 6, 2023
 by @Robjam and @amannn)

There's no need for a new config option, `next-intl` will automatically
pick up the `basePath` you've configured in `next.config.js`. Note
however that you should make sure that you match `/` explicitly in your
middleware `matcher` (see [the updated
docs](https://next-intl-docs.vercel.app/docs/routing/middleware#base-path)).



The only integration points are:
- Apply the `basePath` for redirects, as Next.js doesn't handle this
internally
- Apply the `basePath` for rewrites, as Next.js doesn't handle this
internally
- Apply the `basePath` for alternate links

This PR is based on the initial work in
#589 by @Robjam

Closes #589
Closes #243

---------

Co-authored-by: Robjam <rjames@smartshopping.co.jp>
Co-authored-by: James Roberts <github@robjam.dev>
@amannn
Copy link
Owner Author

amannn commented Dec 6, 2023

Support for basePath was added in next-intl@3.3.0, please see the updated docs.

Many thanks to @Robjam for getting the ball rolling here! 🙌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributions welcome Good for people looking to contribute enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.