From 287ea0c1e5fb2b9091b5e23c3c79fa0e1c2cb8a3 Mon Sep 17 00:00:00 2001
From: Rafal Chlodnicki <rchl2k@gmail.com>
Date: Thu, 22 Jul 2021 13:37:41 +0200
Subject: [PATCH] fix: custom routes with optional params broken by the module

Don't use ufo on Vue Router route definitions as those are not
a normal paths, or not always at least.

Fixes #1242
---
 src/helpers/routes.js               |  5 ++---
 src/helpers/utils.js                | 10 ++++++++++
 test/fixture/basic/extend-routes.js | 13 +++++++++++++
 test/module.test.js                 | 21 +++++++++++++++++----
 4 files changed, 42 insertions(+), 7 deletions(-)
 create mode 100644 test/fixture/basic/extend-routes.js

diff --git a/src/helpers/routes.js b/src/helpers/routes.js
index 738e88e7c..ac45dbf07 100644
--- a/src/helpers/routes.js
+++ b/src/helpers/routes.js
@@ -1,7 +1,6 @@
-import { withoutTrailingSlash, withTrailingSlash } from 'ufo'
 import { STRATEGIES } from './constants'
 import { extractComponentOptions } from './components'
-import { getPageOptions } from './utils'
+import { adjustRouteForTrailingSlash, getPageOptions } from './utils'
 
 /**
  * @typedef {import('@nuxt/types/config/router').NuxtRouteConfig} NuxtRouteConfig
@@ -150,7 +149,7 @@ export function makeRoutes (baseRoutes, {
       // - If "router.trailingSlash" is not specified then default to no trailing slash (like Nuxt)
       // - Children with relative paths must not start with slash so don't append if path is empty.
       if (path.length) { // Don't replace empty (child) path with a slash!
-        path = (trailingSlash ? withTrailingSlash(path, true) : withoutTrailingSlash(path, true)) || (isChildWithRelativePath ? '' : '/')
+        path = adjustRouteForTrailingSlash(path, trailingSlash, isChildWithRelativePath)
       }
 
       if (shouldAddPrefix && isDefaultLocale && strategy === STRATEGIES.PREFIX && includeUprefixedFallback) {
diff --git a/src/helpers/utils.js b/src/helpers/utils.js
index 8b25a0502..4d494de29 100644
--- a/src/helpers/utils.js
+++ b/src/helpers/utils.js
@@ -53,3 +53,13 @@ export function getPageOptions (route, pages, localeCodes, pagesDir, defaultLoca
 
   return options
 }
+
+/**
+ * @param {string} routePath
+ * @param {boolean | undefined} trailingSlash
+ * @param {boolean} [isChildWithRelativePath] Indicates if it is a child route that has relative path
+ * @return {string}
+ */
+export function adjustRouteForTrailingSlash (routePath, trailingSlash, isChildWithRelativePath) {
+  return routePath.replace(/\/+$/, '') + (trailingSlash ? '/' : '') || (isChildWithRelativePath ? '' : '/')
+}
diff --git a/test/fixture/basic/extend-routes.js b/test/fixture/basic/extend-routes.js
new file mode 100644
index 000000000..eec3161bf
--- /dev/null
+++ b/test/fixture/basic/extend-routes.js
@@ -0,0 +1,13 @@
+import { resolve } from 'path'
+
+/** @type {import('@nuxt/types').Module} */
+export default function () {
+  // @ts-ignore
+  this.extendRoutes(function (routes) {
+    routes.push({
+      name: 'custom-route-with-optional-slug',
+      path: '/custom-route/:slug?',
+      component: resolve(__dirname, 'pages/locale.vue')
+    })
+  })
+}
diff --git a/test/module.test.js b/test/module.test.js
index 0ca90e67a..44e93d7f0 100644
--- a/test/module.test.js
+++ b/test/module.test.js
@@ -1,8 +1,9 @@
-import { resolve } from 'path'
+import { join, resolve } from 'path'
 import { readFileSync } from 'fs'
 import { generate, setup, loadConfig, get, url } from '@nuxtjs/module-test-utils'
 import { JSDOM } from 'jsdom'
 import { withoutTrailingSlash, withTrailingSlash } from 'ufo'
+import { adjustRouteForTrailingSlash } from '../src/helpers/utils'
 import { getSeoTags } from './utils'
 
 /**
@@ -280,15 +281,23 @@ for (const trailingSlash of TRAILING_SLASHES) {
       const overrides = {
         router: {
           trailingSlash,
+          // Routes added through extendRoutes are not processed by nuxt-i18n
           extendRoutes (routes) {
             routes.push({
-              path: pathRespectingTrailingSlash('/about'),
-              redirect: pathRespectingTrailingSlash('/about-us')
+              path: adjustRouteForTrailingSlash('/about', trailingSlash),
+              redirect: adjustRouteForTrailingSlash('/about-us', trailingSlash)
             })
           }
         }
       }
-      nuxt = (await setup(loadConfig(__dirname, 'basic', overrides, { merge: true }))).nuxt
+
+      /** @type {import('@nuxt/types').NuxtConfig} */
+      const testConfig = loadConfig(__dirname, 'basic', overrides, { merge: true })
+
+      // Extend routes before the nuxt-i18n module so that the module processes them.
+      testConfig.modules?.unshift(join(__dirname, 'fixture', 'basic', 'extend-routes'))
+
+      nuxt = (await setup(testConfig)).nuxt
     })
 
     afterAll(async () => {
@@ -430,6 +439,10 @@ for (const trailingSlash of TRAILING_SLASHES) {
       await expect(getRespectingTrailingSlash('/es/simple')).rejects.toBeDefined()
     })
 
+    test('navigates to route with optional param without the param specified', async () => {
+      await expect(getRespectingTrailingSlash('/custom-route/')).resolves.toBeDefined()
+    })
+
     describe('posts', () => {
       /** @type {string} */
       let html