Skip to content

Commit

Permalink
Fix 404 URLs on path-based i18n projects
Browse files Browse the repository at this point in the history
  • Loading branch information
blittle committed Feb 7, 2024
1 parent a266436 commit 4f04f7c
Show file tree
Hide file tree
Showing 8 changed files with 135 additions and 10 deletions.
25 changes: 25 additions & 0 deletions .changeset/fifty-stingrays-worry.md
@@ -0,0 +1,25 @@
---
'skeleton': patch
'@shopify/cli-hydrogen': patch
---

This is an important fix to a bug with 404 routes and path-based i18n projects where some unknown routes would not properly render a 404. This fixes all new projects, but to fix existing projects, add a `($locale).tsx` route with the following contents:

```ts
import {type LoaderFunctionArgs} from '@remix-run/server-runtime';

export async function loader({params, context}: LoaderFunctionArgs) {
const {language, country} = context.storefront.i18n;

if (
params.locale &&
params.locale.toLowerCase() !== `${language}-${country}`.toLowerCase()
) {
// If the locale URL param is defined, yet we still are still at the default locale
// then the the locale param must be invalid, send to the 404 page
throw new Response(null, {status: 404});
}

return null;
}
```
4 changes: 4 additions & 0 deletions packages/cli/src/commands/hydrogen/init.test.ts
Expand Up @@ -383,6 +383,7 @@ describe('init', () => {
cwd: getSkeletonSourceDir(),
ignore: ['**/node_modules/**', '**/dist/**'],
});

const resultFiles = await glob('**/*', {cwd: tmpDir});

expect(resultFiles).toEqual(expect.arrayContaining(templateFiles));
Expand Down Expand Up @@ -595,6 +596,9 @@ describe('init', () => {
// Adds locale to the path
expect(resultFiles).toContain('app/routes/($locale)._index.tsx');

// Adds ($locale) route
expect(resultFiles).toContain('app/routes/($locale).ts');

// Injects styles in Root
const serverFile = await readFile(`${tmpDir}/server.ts`);
expect(serverFile).toMatch(/i18n: getLocaleFromRequest\(request\),/);
Expand Down
1 change: 1 addition & 0 deletions packages/cli/src/lib/onboarding/common.ts
Expand Up @@ -143,6 +143,7 @@ export async function handleRouteGeneration(
typescript: language === 'ts',
localePrefix: i18nStrategy === 'subfolders' ? 'locale' : false,
signal: controller.signal,
i18nStrategy,
},
{
rootDirectory: directory,
Expand Down
66 changes: 65 additions & 1 deletion packages/cli/src/lib/setups/routes/generate.ts
@@ -1,4 +1,5 @@
import {readdir} from 'fs/promises';
import {fileURLToPath} from 'node:url';
import {
fileExists,
readFile,
Expand Down Expand Up @@ -31,8 +32,9 @@ import {
import {convertRouteToV1} from '../../../lib/remix-version-interop.js';
import {type RemixConfig, getRemixConfig} from '../../remix-config.js';
import {findFileWithExtension} from '../../file.js';
import {type I18nStrategy} from '../i18n/index.js';

const NO_LOCALE_PATTERNS = [/robots\.txt/];
const NO_LOCALE_PATTERNS = [/robots\.txt/, /\(\$locale\)/];

const ROUTE_MAP = {
home: ['_index', '$'],
Expand Down Expand Up @@ -106,6 +108,7 @@ type GenerateRoutesOptions = Omit<
directory: string;
localePrefix?: GenerateProjectFileOptions['localePrefix'] | false;
v1RouteConvention?: boolean;
i18nStrategy?: I18nStrategy;
};

type RemixConfigParam = Pick<RemixConfig, 'rootDirectory' | 'appDirectory'>;
Expand Down Expand Up @@ -147,6 +150,16 @@ export async function generateRoutes(
);
}

if (options.i18nStrategy === 'subfolders') {
await copyLocaleNamelessRoute({
...options,
typescript,
localePrefix,
appDirectory,
formatOptions,
});
}

return {
routes,
routeGroups,
Expand Down Expand Up @@ -400,3 +413,54 @@ export async function renderRoutePrompt(options?: {abortSignal: AbortSignal}) {

return generateAll ? 'all' : ([] as string[]);
}

async function copyLocaleNamelessRoute({
appDirectory,
typescript,
formatOptions,
}: GenerateProjectFileOptions & {
appDirectory: string;
formatOptions?: FormatOptions;
}) {
return await copyRouteTemplate('($locale)', {
appDirectory,
typescript,
formatOptions,
});
}

async function copyRouteTemplate(
routeTemplateFileName: string,
{
appDirectory,
typescript,
formatOptions,
}: GenerateProjectFileOptions & {
appDirectory: string;
formatOptions?: FormatOptions;
},
) {
const routePath = joinPath(
appDirectory,
'routes',
routeTemplateFileName + (typescript ? '.ts' : '.js'),
);

const templatePath = fileURLToPath(
new URL(`./templates/${routeTemplateFileName}.tsx`, import.meta.url),
);

if (!(await fileExists(templatePath))) {
throw new Error('Unknown strategy');
}

let templateContent = await readFile(templatePath);

if (!typescript) {
templateContent = await transpileFile(templateContent, templatePath);
}

templateContent = await formatCode(templateContent, formatOptions, routePath);

await writeFile(routePath, templateContent);
}
16 changes: 16 additions & 0 deletions packages/cli/src/lib/setups/routes/templates/($locale).tsx
@@ -0,0 +1,16 @@
import {type LoaderFunctionArgs} from '@remix-run/server-runtime';

export async function loader({params, context}: LoaderFunctionArgs) {
const {language, country} = context.storefront.i18n;

if (
params.locale &&
params.locale.toLowerCase() !== `${language}-${country}`.toLowerCase()
) {
// If the locale URL param is defined, yet we still are still at the default locale
// then the the locale param must be invalid, send to the 404 page
throw new Response(null, {status: 404});
}

return null;
}
8 changes: 8 additions & 0 deletions packages/cli/tsup.config.ts
Expand Up @@ -43,6 +43,14 @@ export default defineConfig([
);
console.log('\n', 'Copied i18n template files to build directory', '\n');

// Copy Route Templates
const routeTemplatesPath = 'lib/setups/routes/templates';
await fs.copy(
path.join('src', routeTemplatesPath),
path.join(outDir, routeTemplatesPath),
);
console.log('\n', 'Copied route template files to build directory', '\n');

// Copy Bundle Analyzer
const bundleAnalyzer = 'lib/bundle/bundle-analyzer.html';
await fs.copy(
Expand Down
9 changes: 0 additions & 9 deletions templates/demo-store/app/routes/($locale)._index.tsx
Expand Up @@ -14,15 +14,6 @@ export const headers = routeHeaders;
export async function loader({params, context}: LoaderFunctionArgs) {
const {language, country} = context.storefront.i18n;

if (
params.locale &&
params.locale.toLowerCase() !== `${language}-${country}`.toLowerCase()
) {
// If the locale URL param is defined, yet we still are on `EN-US`
// the the locale param must be invalid, send to the 404 page
throw new Response(null, {status: 404});
}

const {shop, hero} = await context.storefront.query(HOMEPAGE_SEO_QUERY, {
variables: {handle: 'freestyle'},
});
Expand Down
16 changes: 16 additions & 0 deletions templates/demo-store/app/routes/($locale).tsx
@@ -0,0 +1,16 @@
import {type LoaderFunctionArgs} from '@remix-run/server-runtime';

export async function loader({params, context}: LoaderFunctionArgs) {
const {language, country} = context.storefront.i18n;

if (
params.locale &&
params.locale.toLowerCase() !== `${language}-${country}`.toLowerCase()
) {
// If the locale URL param is defined, yet we still are still at the default locale
// then the the locale param must be invalid, send to the 404 page
throw new Response(null, {status: 404});
}

return null;
}

0 comments on commit 4f04f7c

Please sign in to comment.