Conversation
There were a bunch of leftover from experiments. e.g. _lingoConfig was a hack, a field with a config copy added to the config object which can be read by the translation-server cli to parse the arguments. But bundlers were always complaining about it, plus in next with our async setup it would not work. So there is no way to parse the config now. We could add some though if needed, but honestly parsing the file and finding our config section.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
AleksandrSl
changed the title
| @@ -1 +1,2 @@ | |||
| export type { PartialLingoConfig } from "./types"; | |||
| export type { LocaleCode } from "lingo.dev/spec"; | |||
There was a problem hiding this comment.
Makes sense to reexport I think, because it's used in many places, and compiler could be the single dependency.
|
|
||
| export function getLocalePathname(locale) { | ||
| return null; // Not used for cookie-based routing | ||
| } |
There was a problem hiding this comment.
| export function getLocalePathname(locale) { | |
| return null; // Not used for cookie-based routing | |
| } |
| if (router) { | ||
| router.refresh(); | ||
| if (newUrl) { | ||
| router.push(newUrl); |
There was a problem hiding this comment.
I missed that for the path based i18n we will have to do navigation. I don't think we need to add a separate method for that, just returning newUrl form persistLocale if redirect is needed sounds reasonable to me.
Next needs a push, because we have to trigger a middleware so that server components get a correct locale
| } { | ||
| const baseDir = path.join(projectRoot, sourceRoot, lingoDir); | ||
|
|
||
| const serverPath = resolveResolverPath("locale-resolver-server", baseDir); |
There was a problem hiding this comment.
The whole file is a way to check that the file are present where we expect them to be. In this PR I expect them to be in the lingo directory, but we are free to come up with any pattern we want.
Also a nice error if one of the files is missing when custom locale resolver is chosen in settings would be good
| process.cwd(), | ||
| ); | ||
|
|
||
| customResolverAliases = { |
There was a problem hiding this comment.
For vite and unplugin there should already be a similar thing.
See
function tryLocalOrReturnVirtual(
config: LingoConfig,
fileName: string,
virtualName: string,
) {
const customPath = path.join(config.sourceRoot, config.lingoDir, fileName);
if (fs.existsSync(customPath)) {
return customPath;
}
return virtualName;
}
I don't remember if I checked it working
| @@ -0,0 +1,4 @@ | |||
| import type { LocaleCode } from "@lingo.dev/compiler" | |||
There was a problem hiding this comment.
Just a way to avoid repeating these
| @@ -0,0 +1,107 @@ | |||
| import { NextRequest, NextResponse } from "next/server"; | |||
There was a problem hiding this comment.
It's 2026 why the hell Next asks users to configure all this hell when they want path based i18n
1 participant
Summary
It's an example of having custom resolvers and what we have to do to support them. Ofc we have to make path based i18n support internally along with the cookies, without custom resolvers.
There are both changes on the user side (demo) and our side (in the compiler, because I removed the custom resolver part for simplicity until we have requests, but PR also demonstrates how we can support path based i18n, probably it can also be done a bit cleaner, and we can provide middleware configuration as exported function, so users don't have to do it, similar how next-intl does this. suprisingly they have a ton of code around these)
Changes
Testing
Business logic tests added:
Visuals
Required for UI/UX changes:
Checklist
Closes #[issue-number]