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

fix: only validate a locale based on the configuration #845

Merged
merged 3 commits into from
Aug 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/itchy-cooks-tap.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@headstartwp/next": patch
---

Fixed - Fixed isValidLocale function to validate against the configuration
Added - New test condition for technically valid, but unsupported locale
38 changes: 29 additions & 9 deletions packages/next/src/middlewares/__tests__/appMiddleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -520,36 +520,56 @@ describe('appMiddleware', () => {
},
});

let req = new NextRequest('http://test.com/en/post-name', {
const req = new NextRequest('http://test.com/en/post-name', {
method: 'GET',
});

req.headers.set('host', 'test.com');
req.headers.set('accept-language', 'es');

expect(getAppRouterLocale(req)).toStrictEqual(['en', 'en']);
let res = await AppMiddleware(req, { appRouter: true });
const res = await AppMiddleware(req, { appRouter: true });
expect(res.headers.get('x-headstartwp-locale')).toBe('en');
expect(res.headers.get('x-middleware-rewrite')).toBeNull();
// should redirect from /en/post-name to /post-name
expect(res.status).toBe(307);
expect(res.headers.get('Location')).toBe('http://test.com/post-name');
});

it('[polylang] redirects locale that is not supported to browser language url', async () => {
setHeadstartWPConfig({
sourceUrl: 'http://testwp.com',
hostUrl: 'http://test.com',
integrations: {
polylang: {
enable: true,
},
},
i18n: {
locales: ['en', 'es'],
defaultLocale: 'en',
},
});

// pt is a unsuported but valid
// the it should not redirect but just let it 404
req = new NextRequest('http://test.com/pt/post-name', {
// pt is unsupported
// it should redirect to a url with the browser's language
const req = new NextRequest('http://test.com/pt/post-name', {
method: 'GET',
});

req.headers.set('host', 'test.com');
req.headers.set('accept-language', 'es');

expect(getAppRouterLocale(req)).toStrictEqual(['en', 'pt']);
res = await AppMiddleware(req, { appRouter: true });
expect(res.headers.get('x-headstartwp-locale')).toBe('pt');
// Will match the default, then the accept-language header for a locale that is unsupported
expect(getAppRouterLocale(req)).toStrictEqual(['en', 'es']);
const res = await AppMiddleware(req, { appRouter: true });

// Will match the accept-language header for a locale that is unsupported
expect(res.headers.get('x-headstartwp-locale')).toBe('es');
expect(res.headers.get('x-middleware-rewrite')).toBeNull();

expect(res.status).toBe(200);
expect(res.status).toBe(307);
expect(res.headers.get('Location')).toBe('http://test.com/es/pt/post-name');
});

it('[multisite with locale] gets locale from url if set', async () => {
Expand Down
16 changes: 7 additions & 9 deletions packages/next/src/middlewares/appMidleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,6 @@ function isPolylangIntegrationEnabled() {
return config.integrations?.polylang?.enable ?? false;
}

function isValidLocale(locale: string) {
try {
return Intl.getCanonicalLocales(locale).length > 0;
} catch (e) {
return false;
}
}

function getAppRouterSupportedLocales() {
const config = getHeadstartWPConfig();

Expand All @@ -43,6 +35,12 @@ function getAppRouterSupportedLocales() {
};
}

function isValidLocale(locale: string) {
const { supportedLocales } = getAppRouterSupportedLocales();

return supportedLocales.includes(locale);
}

/**
* On App Router it returns a tuple with defaultLocale and locale
*
Expand All @@ -62,7 +60,7 @@ export function getAppRouterLocale(request: NextRequest): [string, string] | und

// if there's a locale in the URL and it's a supported or valid locale, use it
const urlLocale = request.nextUrl.pathname.split('/')[1];
if (supportedLocales.includes(urlLocale) || isValidLocale(urlLocale)) {
if (isValidLocale(urlLocale)) {
return [defaultLocale, urlLocale];
}

Expand Down
Loading