Fix 404 URLs on path-based i18n projects (#1732)

* Fix 404 URLs on path-based i18n projects --------- Co-authored-by: Fran Dios <fran.dios@shopify.com>
Shopify · Feb 8, 2024 · fcecfb2 · fcecfb2
1 parent abb7137
commit fcecfb2
Show file tree

Hide file tree

Showing 11 changed files with 187 additions and 20 deletions.
diff --git a/.changeset/fifty-stingrays-worry.md b/.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;
+}
+```
diff --git a/packages/cli/oclif.manifest.json b/packages/cli/oclif.manifest.json
@@ -777,6 +777,12 @@
           "description": "Generate TypeScript files",
           "allowNo": false
         },
+        "locale-param": {
+          "name": "locale-param",
+          "type": "option",
+          "description": "The param name in Remix routes for the i18n locale, if any. Example: `locale` becomes ($locale).",
+          "multiple": false
+        },
         "force": {
           "name": "force",
           "type": "boolean",
@@ -834,6 +840,12 @@
           "description": "Generate TypeScript files",
           "allowNo": false
         },
+        "locale-param": {
+          "name": "locale-param",
+          "type": "option",
+          "description": "The param name in Remix routes for the i18n locale, if any. Example: `locale` becomes ($locale).",
+          "multiple": false
+        },
         "force": {
           "name": "force",
           "type": "boolean",

diff --git a/packages/cli/remix.env.d.ts b/packages/cli/remix.env.d.ts
@@ -0,0 +1,20 @@
+/// <reference types="@shopify/remix-oxygen" />
+
+import type {
+  storefront,
+  customeraccount,
+  hydrogencart,
+} from '@shopify/hydrogen';
+
+declare module '@shopify/remix-oxygen' {
+  /**
+   * Declare local additions to the Remix loader context.
+   */
+  export interface AppLoadContext {
+    env: Env;
+    cart: HydrogenCart;
+    storefront: Storefront;
+    customerAccount: CustomerAccount;
+    waitUntil: ExecutionContext['waitUntil'];
+  }
+}
diff --git a/packages/cli/src/commands/hydrogen/generate/route.ts b/packages/cli/src/commands/hydrogen/generate/route.ts
@@ -26,6 +26,11 @@ export default class GenerateRoute extends Command {
       description: 'Generate TypeScript files',
       env: 'SHOPIFY_HYDROGEN_FLAG_TYPESCRIPT',
     }),
+    'locale-param': Flags.string({
+      description:
+        'The param name in Remix routes for the i18n locale, if any. Example: `locale` becomes ($locale).',
+      env: 'SHOPIFY_HYDROGEN_FLAG_ADAPTER',
+    }),
     force: commonFlags.force,
     path: commonFlags.path,
   };
@@ -54,6 +59,7 @@ export default class GenerateRoute extends Command {
       ...flags,
       directory,
       routeName,
+      localePrefix: flags['locale-param'],
     });
   }
 }
@@ -64,6 +70,7 @@ export async function runGenerate(options: {
   adapter?: string;
   typescript?: boolean;
   force?: boolean;
+  localePrefix?: string;
 }) {
   const {routes} = await generateRoutes({
     ...options,

diff --git a/packages/cli/src/commands/hydrogen/init.test.ts b/packages/cli/src/commands/hydrogen/init.test.ts
@@ -383,6 +383,7 @@ describe('init', () => {
           cwd: getSkeletonSourceDir(),
           ignore: ['**/node_modules/**', '**/dist/**'],
         });
+
         const resultFiles = await glob('**/*', {cwd: tmpDir});
 
         expect(resultFiles).toEqual(expect.arrayContaining(templateFiles));
@@ -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).tsx');
+
           // Injects styles in Root
           const serverFile = await readFile(`${tmpDir}/server.ts`);
           expect(serverFile).toMatch(/i18n: getLocaleFromRequest\(request\),/);

diff --git a/packages/cli/src/lib/setups/routes/generate.ts b/packages/cli/src/lib/setups/routes/generate.ts
@@ -1,4 +1,5 @@
-import {readdir} from 'fs/promises';
+import {readdir} from 'node:fs/promises';
+import {fileURLToPath} from 'node:url';
 import {
   fileExists,
   readFile,
@@ -127,7 +128,8 @@ export async function generateRoutes(
   );
 
   const formatOptions = await getCodeFormatOptions(rootDirectory);
-  const localePrefix = await getLocalePrefix(appDirectory, options);
+  const routesDirectory = joinPath(appDirectory, GENERATOR_ROUTE_DIR);
+  const localePrefix = await getLocalePrefix(routesDirectory, options);
   const typescript = !!(
     options.typescript ??
     (await fileExists(joinPath(rootDirectory, 'tsconfig.json')))
@@ -147,6 +149,16 @@ export async function generateRoutes(
     );
   }
 
+  if (localePrefix) {
+    await copyLocaleNamelessRoute({
+      typescript,
+      localePrefix,
+      routesDirectory,
+      formatOptions,
+      adapter: options.adapter,
+    });
+  }
+
   return {
     routes,
     routeGroups,
@@ -170,15 +182,13 @@ type GenerateProjectFileOptions = {
  * In V2, we check the home route for the presence of `($...)._index` in the filename.
  */
 async function getLocalePrefix(
-  appDirectory: string,
+  routesDirectory: string,
   {localePrefix, routeName, v1RouteConvention}: GenerateRoutesOptions,
 ) {
   if (localePrefix) return localePrefix;
   if (localePrefix !== undefined || routeName === 'all') return;
 
-  const existingFiles = await readdir(joinPath(appDirectory, 'routes')).catch(
-    () => [],
-  );
+  const existingFiles = await readdir(routesDirectory).catch(() => []);
 
   const coreRouteWithLocaleRE = v1RouteConvention
     ? /^\(\$(\w+)\)$/
@@ -290,10 +300,7 @@ export async function generateProjectFile(
     // If the command was run with an adapter flag, we replace the default
     // import with the adapter that was passed.
     if (adapter) {
-      templateContent = templateContent.replace(
-        /@shopify\/remix-oxygen/g,
-        adapter,
-      );
+      templateContent = replaceAdapters(templateContent, adapter);
     }
 
     // We format the template content with Prettier.
@@ -312,6 +319,10 @@ export async function generateProjectFile(
   return result;
 }
 
+function replaceAdapters(templateContent: string, adapter: string) {
+  return templateContent.replace(/@shopify\/remix-oxygen/g, adapter);
+}
+
 /**
  * Find the destination path for a route file in the user project.
  */
@@ -400,3 +411,60 @@ export async function renderRoutePrompt(options?: {abortSignal: AbortSignal}) {
 
   return generateAll ? 'all' : ([] as string[]);
 }
+
+function copyLocaleNamelessRoute({
+  typescript,
+  localePrefix,
+  ...options
+}: {localePrefix: string} & Omit<
+  RouteTemplateOptions,
+  'templateName' | 'routeName'
+>) {
+  return copyRouteTemplate({
+    ...options,
+    typescript,
+    templateName: 'locale-check.ts',
+    routeName: `(\$${localePrefix})${typescript ? '.tsx' : '.jsx'}`,
+  });
+}
+
+type RouteTemplateOptions = {
+  routesDirectory: string;
+  templateName: string;
+  routeName: string;
+  formatOptions?: FormatOptions;
+} & Pick<GenerateProjectFileOptions, 'adapter' | 'typescript'>;
+
+async function copyRouteTemplate({
+  templateName,
+  routeName,
+  routesDirectory,
+  formatOptions,
+  typescript,
+  adapter,
+}: RouteTemplateOptions) {
+  const routePath = joinPath(routesDirectory, routeName);
+  if (await fileExists(routePath)) return;
+
+  const templatePath = fileURLToPath(
+    new URL(`./templates/${templateName}`, import.meta.url),
+  );
+
+  if (!(await fileExists(templatePath))) {
+    throw new Error('Unknown strategy');
+  }
+
+  let templateContent = await readFile(templatePath);
+
+  if (adapter) {
+    templateContent = replaceAdapters(templateContent, adapter);
+  }
+
+  if (!typescript) {
+    templateContent = await transpileFile(templateContent, templatePath);
+  }
+
+  templateContent = await formatCode(templateContent, formatOptions, routePath);
+
+  await writeFile(routePath, templateContent);
+}
diff --git a/packages/cli/src/lib/setups/routes/templates/locale-check.ts b/packages/cli/src/lib/setups/routes/templates/locale-check.ts
@@ -0,0 +1,16 @@
+import type {LoaderFunctionArgs} from '@shopify/remix-oxygen';
+
+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;
+}
diff --git a/packages/cli/tsconfig.json b/packages/cli/tsconfig.json
@@ -1,6 +1,6 @@
 {
   "extends": "../../tsconfig.json",
-  "include": ["src"],
+  "include": ["src", "remix.env.d.ts"],
   "exclude": ["dist", "__tests__", "node_modules", ".turbo"],
   "compilerOptions": {
     "module": "NodeNext",

diff --git a/packages/cli/tsup.config.ts b/packages/cli/tsup.config.ts
@@ -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(

diff --git a/templates/demo-store/app/routes/($locale)._index.tsx b/templates/demo-store/app/routes/($locale)._index.tsx
@@ -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'},
   });

diff --git a/templates/demo-store/app/routes/($locale).tsx b/templates/demo-store/app/routes/($locale).tsx
@@ -0,0 +1,16 @@
+import type {LoaderFunctionArgs} from '@shopify/remix-oxygen';
+
+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;
+}