Fix navigation using prefixed urls

hozana · Nov 2, 2022 · 7cdf200 · 7cdf200
1 parent 539b431
commit 7cdf200
Show file tree

Hide file tree

Showing 8 changed files with 239 additions and 104 deletions.
diff --git a/README.md b/README.md
@@ -8,7 +8,7 @@ Translated routing and more for Next using Next regular file-base routing system
   - [Basic usage](#basic-usage)
     1. [Wrap you next config with the next-translate-routes plugin](#1-wrap-you-next-config-with-the-next-translate-routes-plugin)
     2. [Define your routes](#2-define-your-routes)
-    3. [Wrap your `\_app` component with the `withTranslateRoutes` hoc](#3-wrap-you-app-component-with-the-withtranslateroutes-hoc)
+    3. [Wrap your `\_app` component with the `withTranslateRoutes` hoc](#3-wrap-you-_app-component-with-the-withtranslateroutes-hoc)
     4. [Use `next-translate-routes/link` instead of `next/link`](#4-use-next-translate-routeslink-instead-of-nextlink)
     5. [Use `next-translate-routes/router instead` of `next/router` for singleton router (default export)](#5-use-next-translate-routesrouter-instead-of-nextrouter-for-singleton-router-default-export)
   - [Advanced usage](#advanced-usage)
@@ -22,16 +22,16 @@ Translated routing and more for Next using Next regular file-base routing system
     - [Custom route tree](#custom-route-tree)
     - [Outside Next](#outside-next)
 - [Known issue](#known-issues)
-  - [Middleware with Next >=12.2.0](#middleware-with-next-1220)
+  - [Middleware with Next >=12.2.0](#middleware-with-watcher-next-1220)
 - [How does it work](#how-does-it-work)
 
 ## Features
 
 - **Translated paths segments**  
   Ex: `/contact-us` and `/fr/contactez-nous`.
-- **Complex paths segments** (path-to-regexp synthax)  
+- **Complex paths segments** (path-to-regexp syntax)  
   Ex: `/:id{-:slug}?/`
-- **Constrained dynamic paths segments** (path-to-regexp synthax)  
+- **Constrained dynamic paths segments** (path-to-regexp syntax)  
   Ex: `/:id(\\d+)/`
 - **No duplicated content (SEO)**  
   Simple rewrites would make a page accessible with 2 different urls which is bad for SEO.
@@ -257,6 +257,8 @@ If `routesDataFileName` is defined, to `'routesData'` for example, next-translat
 
 If `routesTree` is defined, next-translate-routes won't parse the `pages` folder and will use the given object as the routes tree. If you uses it, beware of building correctly the routes tree to avoid bugs.
 
+You can see and edit these while your app is running to debug things, using `__NEXT_TRANSLATE_ROUTES_DATA` in the browser console. For exemple, executing `__NEXT_TRANSLATE_ROUTES_DATA.debug = true` will activate the logs on `router.push` and `router.replace`.
+
 #### Translate/untranslate urls
 
 Two helpers are exposed to translate/untranslate urls:
@@ -280,7 +282,7 @@ You will probably want to indicate alternate pages for SEO optimization. Here is
   )
 ```
 
-You can do it in the `_app` component if you are sure to do that for all your pages.
+You can do it in the `_app` component if you are sure to do that for all your pages. You can also use a dedicated package, like [next-seo](https://github.com/garmeeh/next-seo).
 
 See [this article about alternate and canonical pages](https://hreflang.org/use-hreflang-canonical-together/)
 
@@ -500,14 +502,21 @@ module.exports = ({ config }) => {
 
 ### Middleware with watcher (Next >=12.2.0)
 
-Unfortunately, Next new middleware synthax (stable) has a bug when using a "matcher" and rewrites.
+Unfortunately, Next new middleware syntax (stable) has a bug when using a "matcher" and rewrites.
 You can keep track of this issue:
 
 - [in next-translate-routes repo](https://github.com/hozana/next-translate-routes/issues/19)
 - [in next.js repo](https://github.com/vercel/next.js/issues/39531)
 
 So if you want to use a middleware with Next >= 12.2.0, you need to remove any watcher option and filter from within the middleware using [conditional statements](https://nextjs.org/docs/advanced-features/middleware#conditional-statements).
 
+### Optional catch-all with rewrites
+
+Another bug in Next.js mess some optional catch all routes when they are rewritten.
+You can keep track of this issue:
+
+- [in the next.js issue](https://github.com/vercel/next.js/issues/41624)
+
 ### Domain routing
 
 Domain routing is not supported yet but should be in the future. Any PR to make it work are welcome.

diff --git a/src/plugin/fileNameToPaths.ts b/src/plugin/fileNameToPaths.ts
@@ -1,12 +1,12 @@
 import {
   matchAllFilepathPartsRegex,
-  optionalMatchAllFilepathPartsRegex,
+  optionalMatchAllFilepathPartRegex,
   dynamicFilepathPartsRegex,
 } from '../shared/regex'
 
-/** Transform Next file-system synthax to path-to-regexp synthax */
+/** Transform Next file-system syntax to path-to-regexp syntax */
 export const fileNameToPath = (fileName: string) =>
   fileName
-    .replace(optionalMatchAllFilepathPartsRegex, ':$1*') // [[...param]]
+    .replace(optionalMatchAllFilepathPartRegex, ':$1*') // [[...param]]
     .replace(matchAllFilepathPartsRegex, ':$1+') // [...param]
     .replace(dynamicFilepathPartsRegex, ':$1') // [param]
diff --git a/src/react/enhanceNextRouter.ts b/src/react/enhanceNextRouter.ts
@@ -4,9 +4,9 @@ import { stringify as stringifyQuery } from 'querystring'
 
 import { ntrMessagePrefix } from '../shared/withNtrPrefix'
 import type { Url } from '../types'
-import { fileUrlToUrl } from './fileUrlToUrl'
 import { getLocale } from './getLocale'
 import { getNtrData } from './ntrData'
+import { translatePushReplaceArgs } from './translatePushReplaceArgs'
 import { urlToFileUrl } from './urlToFileUrl'
 
 interface Options {
@@ -24,28 +24,20 @@ const logWithTrace = (from: string, details: unknown) => {
 const enhancePushReplace =
   <R extends NextRouter | SingletonRouter>(router: R, fnName: 'push' | 'replace') =>
   (url: Url, as?: Url, options?: Options) => {
-    const locale = getLocale(router, options?.locale)
-    const parsedUrl = typeof url === 'string' ? urlToFileUrl(url, locale) : url
-    let translatedUrl = as
-    if (!as && parsedUrl) {
-      try {
-        translatedUrl = fileUrlToUrl(parsedUrl, locale)
-      } catch {
-        // Url is wrong
-      }
-    }
+    const translatedArgs = translatePushReplaceArgs({ router, url, as, locale: options?.locale })
+
     if (getNtrData().debug) {
       logWithTrace(`router.${fnName}`, {
-        url,
-        as,
-        options,
-        translatedUrl,
-        parsedUrl,
-        locale,
+        original: {
+          url,
+          as,
+          options,
+        },
+        translated: translatedArgs,
       })
     }
 
-    return router[fnName](parsedUrl || url, translatedUrl, options)
+    return router[fnName](translatedArgs.url, translatedArgs.as, { ...options, locale: translatedArgs.locale })
   }
 
 const enhancePrefetch =

diff --git a/src/react/fileUrlToUrl.ts b/src/react/fileUrlToUrl.ts
@@ -2,12 +2,23 @@ import { normalizePathTrailingSlash } from 'next/dist/client/normalize-trailing-
 import { parse as parsePathPattern, compile as compilePath } from 'path-to-regexp'
 import { format as formatUrl, UrlObject } from 'url'
 
-import { ignoreSegmentPathRegex } from '../shared/regex'
+import {
+  anyDynamicFilepathPartRegex,
+  dynamicFilepathPartsRegex,
+  getDynamicPathPartKey,
+  ignoreSegmentPathRegex,
+  optionalMatchAllFilepathPartRegex,
+  spreadFilepathPartRegex,
+} from '../shared/regex'
 import { ntrMessagePrefix } from '../shared/withNtrPrefix'
 import type { TRouteBranch } from '../types'
 import { getNtrData } from './ntrData'
 import { urlToUrlObject } from './urlToUrlObject'
 
+/**
+ * Get pattern from route branch paths property in the specified locale,
+ * or, if it does not exist, the default path.
+ */
 const getPatternFromRoutePaths = (routeBranch: TRouteBranch, locale: string) => {
   const pattern = routeBranch.paths[locale] || routeBranch.paths.default
   return ignoreSegmentPathRegex.test(pattern) ? '' : pattern
@@ -29,11 +40,14 @@ const getPathPatternPart = ({
 
   if (isLastPathPart) {
     if (routeBranch.children?.length) {
-      const indexChild = routeBranch.children.find((child) => child.name === 'index' && !child.children?.length)
-      if (!indexChild) {
+      const silentChild = routeBranch.children.find(
+        (child) =>
+          !child.children?.length && (child.name === 'index' || optionalMatchAllFilepathPartRegex.test(child.name)),
+      )
+      if (!silentChild) {
         throw new Error(`No index file found in "${routeBranch.name}" folder.`)
       }
-      const indexChildPattern = getPatternFromRoutePaths(indexChild, locale)
+      const indexChildPattern = getPatternFromRoutePaths(silentChild, locale)
       if (indexChildPattern && indexChildPattern !== 'index') {
         pattern = pattern + addSlashPrefix(indexChildPattern)
       }
@@ -46,9 +60,11 @@ const getPathPatternPart = ({
 }
 
 /**
- * Get path pattern (path-to-regexp synthax) from file path parts
+ * Recursively get path pattern (path-to-regexp syntax) from file path parts
  *
- * Ex: `/[dynamic]/path` => `/:dynamic/path`
+ * Ex, given `/[dynamic]/path` is an existing file path:
+ * `/[dynamic]/path` => { pattern: `/:dynamic/path`, values: {} }
+ * `/value/path` => { pattern: `/:dynamic/path`, values: { dynamic: 'value' } }
  */
 export const getTranslatedPathPattern = ({
   routeBranch,
@@ -59,35 +75,73 @@ export const getTranslatedPathPattern = ({
   /** Remaining path parts after the `routeBranch` path parts */
   pathParts: string[]
   locale: string
-}): string => {
+}): { pattern: string; values: Record<string, string | string[]> } => {
   const isLastPathPart = pathParts.length === 0
 
+  /** Current part path pattern */
   const currentPathPatternPart = getPathPatternPart({ routeBranch, locale, isLastPathPart })
 
   if (isLastPathPart) {
-    return currentPathPatternPart
+    return { pattern: currentPathPatternPart, values: {} }
   }
 
   const nextPathPart = pathParts[0]
   const remainingPathParts = pathParts.slice(1)
+  const hasNextPathPartDynamicSyntax = anyDynamicFilepathPartRegex.test(nextPathPart)
+
+  // Next parts path patterns: looking for the child corresponding to nextPathPart:
+  // if nextPathPart does not match any child name and a dynamic child is found,
+  // we will consider that nextPathPart is a value given to the dynamic child
+
+  let matchingChild: TRouteBranch | undefined = undefined
 
   for (const child of routeBranch.children || []) {
     if (
-      // child name must match nextPathPart
-      child.name === nextPathPart &&
       // child.children must be coherent with remaining path parts is case a file and and folder share the same name
-      (remainingPathParts.length === 0 || child.children?.length)
+      remainingPathParts.length === 0 ||
+      child.children?.length
     ) {
-      return (
-        currentPathPatternPart +
-        getTranslatedPathPattern({
-          routeBranch: child,
-          pathParts: remainingPathParts,
-          locale,
-        })
-      )
+      if (child.name === nextPathPart) {
+        matchingChild = child
+        break
+      } else if (
+        // If nextPathPart already have a dynamic syntax, it must match the name, no need to go further
+        !hasNextPathPartDynamicSyntax &&
+        // If the current child is dynamic and...
+        anyDynamicFilepathPartRegex.test(child.name) &&
+        // ...there is no matching child found for now, or...
+        (!matchingChild ||
+          // ...the matchingChild has a sread syntax and the new one has not (priority)
+          (spreadFilepathPartRegex.test(matchingChild.name) && dynamicFilepathPartsRegex.test(child.name)))
+      ) {
+        matchingChild = child
+      }
     }
   }
+
+  if (matchingChild) {
+    /** If we found an exact match, no need to add values */
+    const isExactMatch = matchingChild.name === nextPathPart
+    const dynamicPathPartKey = getDynamicPathPartKey(matchingChild.name)
+
+    const { pattern: nextPattern, values: nextValues } = getTranslatedPathPattern({
+      routeBranch: matchingChild,
+      pathParts: remainingPathParts,
+      locale,
+    })
+
+    const pattern = currentPathPatternPart + nextPattern
+    const values =
+      isExactMatch || !dynamicPathPartKey
+        ? nextValues
+        : {
+            [dynamicPathPartKey]: spreadFilepathPartRegex.test(matchingChild.name) ? pathParts : nextPathPart,
+            ...nextValues,
+          }
+
+    return { pattern, values }
+  }
+
   throw new Error(`No "/${pathParts.join('/')}" page found in /${routeBranch.name} folder.`)
 }
 
@@ -112,11 +166,10 @@ export const fileUrlToUrl = (url: UrlObject | URL | string, locale: string) => {
       .split('/')
       .filter(Boolean)
 
-    // Create a new query object that we can mutate
-    const newQuery = { ...query }
+    const { pattern: pathPattern, values } = getTranslatedPathPattern({ routeBranch: routesTree, pathParts, locale })
 
-    const pathPattern = getTranslatedPathPattern({ routeBranch: routesTree, pathParts, locale })
-    const newPathname = normalizePathTrailingSlash(compilePath(pathPattern)(query))
+    const newQuery = { ...query, ...values }
+    const newPathname = normalizePathTrailingSlash(compilePath(pathPattern)(newQuery))
 
     for (const patterToken of parsePathPattern(pathPattern)) {
       if (typeof patterToken === 'object' && patterToken.name) {

diff --git a/src/react/link.tsx b/src/react/link.tsx
@@ -1,60 +1,17 @@
 import NextLink, { LinkProps } from 'next/link'
 import { useRouter as useNextRouter } from 'next/router'
 import React from 'react'
-import type { UrlObject } from 'url'
 
-import { fileUrlToUrl } from './fileUrlToUrl'
-import { getLocale } from './getLocale'
-import { getNtrData } from './ntrData'
-import { removeLangPrefix } from './removeLangPrefix'
-import { urlToFileUrl } from './urlToFileUrl'
+import { translatePushReplaceArgs } from './translatePushReplaceArgs'
 
 /**
  * Link component that handle route translations
  */
-export const Link: React.FC<React.PropsWithChildren<LinkProps>> = ({ href, as, locale: propLocale, ...props }) => {
+export const Link: React.FC<React.PropsWithChildren<LinkProps>> = ({ href, as, locale, ...props }) => {
   const router = useNextRouter()
-  let locale = getLocale(router, propLocale)
-  const locales = router.locales || getNtrData().locales
-  const unPrefixedHref = typeof href === 'string' ? removeLangPrefix(href) : href
+  const translatedArgs = translatePushReplaceArgs({ router, url: href, as, locale })
 
-  if (!propLocale && typeof href === 'string' && unPrefixedHref !== href) {
-    const hrefLocale = href.split('/')[1]
-    if (hrefLocale && locales.includes(hrefLocale)) {
-      locale = hrefLocale
-    }
-  }
-
-  let translatedUrl: string | undefined
-  let parsedUrl: UrlObject | URL | string | undefined
-
-  /**
-   * Href can be:
-   * - an external url
-   * - a correct file url
-   * - a wrong file url (not matching any page)
-   * - a correct translated url
-   * - a wrong translated url
-   */
-  try {
-    translatedUrl = fileUrlToUrl(unPrefixedHref, locale)
-    // Href is a correct file url
-    parsedUrl = unPrefixedHref
-  } catch {
-    parsedUrl = urlToFileUrl(unPrefixedHref, locale)
-    if (parsedUrl) {
-      try {
-        translatedUrl = fileUrlToUrl(parsedUrl, locale)
-        // Href is a correct translated url
-      } catch {
-        // Href is a wrong file url or an external url
-      }
-    } else {
-      // Href is a wrong url or an external url
-    }
-  }
-
-  return <NextLink href={parsedUrl || href} as={as || translatedUrl} locale={locale} {...props} />
+  return <NextLink href={translatedArgs.url} as={translatedArgs.as} locale={translatedArgs.locale} {...props} />
 }
 
 export default Link