Repository: nuxt/nuxt.js
Pull request #6331 (2019-08-29 00:54:22)

feat(builder, router): enforce uniform trailing slash handling
## Types of changes
- [ ] Bug fix (a non-breaking change which fixes an issue)
- [x] New feature (a non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)


## Description

Trailing slash handling is important when it comes to SEO. For every existing Nuxt application, the content provided by /route and /route/ should be the same (opposed to how it sometimes worked back in the days). But when it comes to how the search engine sees our page, we have to be consistent and choose one of the options, either with or without a trailing slash.

> Be consistent with the preferred version. Use it in your internal links. If you have a Sitemap, include the preferred version (and don’t include the duplicate URL).

With the current setup, it is not entirely possible to enforce trailing slashes (or the removal of them) without substantial effort.

As long as you have simple and fixed URLs, you can always append the slash (or not), but when it comes to resolved named routes (e.g. when using https://github.com/nuxt-community/nuxt-i18n/ ), things become difficult and hard to manage.

**Thus, this PR**

---

It introduces a new router option called `trailingSlashes` set to `undefined` by default. If it's set to `true`, trailing slashes will be appended on every route. If set to `false`, they'll be removed.

Therefore, we use the `pathToRegexpOptions` settings (per route) provided via vue-router (see https://github.com/vuejs/vue-router/issues/1753 and https://github.com/vuejs/vue-router/issues/1443)

---

This option should not be taken lightly and has to be tested thoroughly. When setting `router.trailingSlashes` to something else than undefined, one of the options (either with or w/o trailing slash) will stop working. 301 redirects should be in place before.

---


Related work:

https://github.com/nuxt/nuxt.js/pull/3448/
https://github.com/nuxt/nuxt.js/pull/5746


## TODO

- [ ] check edge cases
	- [ ] index route
    - [ ] nuxt child routes 
	- [ ] (???)
- [ ] docs

## Checklist:
- [ ] My change requires a change to the documentation.
- [ ] I have updated the documentation accordingly. (PR: #)
- [ ] I have added tests to cover my changes (if not applicable, please state why)
- [ ] All new and existing tests are passing.



codecov-io: # [Codecov](https://codecov.io/gh/nuxt/nuxt.js/pull/6331?src=pr&el=h1) Report
> Merging [#6331](https://codecov.io/gh/nuxt/nuxt.js/pull/6331?src=pr&el=desc) into [dev](https://codecov.io/gh/nuxt/nuxt.js/commit/c8ee9a660809e856c28d8678c6a632bbdd6ed00f?src=pr&el=desc) will **increase** coverage by `0.01%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/nuxt/nuxt.js/pull/6331/graphs/tree.svg?width=650&token=nGD1PtMB3M&height=150&src=pr)](https://codecov.io/gh/nuxt/nuxt.js/pull/6331?src=pr&el=tree)

```diff
@@            Coverage Diff            @@
##             dev    #6331      +/-   ##
=========================================
+ Coverage   95.7%   95.71%   +0.01%     
=========================================
  Files         79       79              
  Lines       2652     2659       +7     
  Branches     683      688       +5     
=========================================
+ Hits        2538     2545       +7     
  Misses        98       98              
  Partials      16       16
```

| Flag | Coverage Δ | |
|---|---|---|
| #e2e | `100% <ø> (ø)` | :arrow_up: |
| #fixtures | `50.58% <57.14%> (+0.01%)` | :arrow_up: |
| #unit | `92.36% <100%> (+0.05%)` | :arrow_up: |

| [Impacted Files](https://codecov.io/gh/nuxt/nuxt.js/pull/6331?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [packages/config/src/config/router.js](https://codecov.io/gh/nuxt/nuxt.js/pull/6331/diff?src=pr&el=tree#diff-cGFja2FnZXMvY29uZmlnL3NyYy9jb25maWcvcm91dGVyLmpz) | `100% <ø> (ø)` | :arrow_up: |
| [packages/utils/src/route.js](https://codecov.io/gh/nuxt/nuxt.js/pull/6331/diff?src=pr&el=tree#diff-cGFja2FnZXMvdXRpbHMvc3JjL3JvdXRlLmpz) | `100% <100%> (ø)` | :arrow_up: |
| [packages/builder/src/builder.js](https://codecov.io/gh/nuxt/nuxt.js/pull/6331/diff?src=pr&el=tree#diff-cGFja2FnZXMvYnVpbGRlci9zcmMvYnVpbGRlci5qcw==) | `99.62% <100%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/nuxt/nuxt.js/pull/6331?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/nuxt/nuxt.js/pull/6331?src=pr&el=footer). Last update [c8ee9a6...ba2ceef](https://codecov.io/gh/nuxt/nuxt.js/pull/6331?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

pimlie: Nice timing, have been struggling with trailing slashes in nuxt/press a bit the last weeks!

I'd recommend to include a util method so users can easliy normalize paths they use in their project as well, see eg [here](https://github.com/nuxt/press/blob/develop/src/utils/route.js) for the methods currently used in nuxt/press (but they could be improved on probably).
Atinux: Good call!
Shouldn’t be easier to directly add a meta canonical directly so we fix the duplicate for SEO? (This is what we use for nuxtjs.org)
manniL: >  Shouldn’t be easier to directly add a meta canonical directly so we fix the duplicate for SEO? (This is what we use for nuxtjs.org)

Well, canonical is one thing and easily realizable via layouts ([example](https://github.com/Developmint/developmint.de/blob/master/layouts/default.vue#L35-L49)). But the internal link structure should reflect trailing slashes (or none) as well 
manniL: > I'd recommend to include a util method so users can easliy normalize paths they use in their project as well

What would be the use case when all (named) routes or those resolved programmatically will go in line with the config and fixed routes would be easy to change by appending/removing the `/`?


pimlie: @Atinux Isnt that technically more a workaround then a fix? Due to the nature of a SPA the path resolve behaviour is just different then serving static files with a webserver (ie apache/nginx). How close do we want to stay to that 'original' behaviour?

@manniL If your router config lists all routes with a trailing slash, then your nuxt/router-links should point to a link with a trailing slash as well right?
manniL: > If your router config lists all routes with a trailing slash, then your nuxt/router-links should point to a link with a trailing slash as well right?

Correct. If you use fixed URLs as in `to="/simple-string/"` you can do it on your own. If you used named routes `to="{ name: 'foobar' }"` it is handled by vue-router already and will include the trailing slash (or not)
pimlie: > If you use fixed URLs as in `to="/simple-string/"` you can do it on your own

Thanks, this is why I recommend a normalization utility. Maybe even within nuxt-link itself, but should also be accessible outside for easy use in eg the sitemap-module.

If we support this feature switching between having a trailing slash or not should be as effortless as possible, preferable even with just toggling the setting if you use the normalization utility everywhere.


manniL: > Thanks, this is why I recommend a normalization utility. Maybe even within nuxt-link itself, but should also be accessible outside for easy use in eg the sitemap-module.

Now I got it, that'd be a nice DX improvement and also guarantee that all modules implement the same normalization.

A utility directly included in the `nuxt-link` component is also a nice idea, but could be more difficult than it seems on the first look (taking e.g. query params and programmatic usage into consideration)



Patch:
diff --git a/packages/builder/src/builder.js b/packages/builder/src/builder.js
index 64449a698..39a055d37 100644
--- a/packages/builder/src/builder.js
+++ b/packages/builder/src/builder.js
@@ -337,12 +337,14 @@ export default class Builder {
 
   async resolveRoutes ({ templateVars }) {
     consola.debug('Generating routes...')
+    const { routeNameSplitter, trailingSlash } = this.options.router
 
     if (this._defaultPage) {
       templateVars.router.routes = createRoutes({
         files: ['index.vue'],
         srcDir: this.template.dir + '/pages',
-        routeNameSplitter: this.options.router.routeNameSplitter
+        routeNameSplitter,
+        trailingSlash
       })
     } else if (this._nuxtPages) {
       // Use nuxt.js createRoutes bases on pages/
@@ -359,8 +361,9 @@ export default class Builder {
         files: Object.values(files),
         srcDir: this.options.srcDir,
         pagesDir: this.options.dir.pages,
-        routeNameSplitter: this.options.router.routeNameSplitter,
-        supportedExtensions: this.supportedExtensions
+        routeNameSplitter,
+        supportedExtensions: this.supportedExtensions,
+        trailingSlash
       })
     } else { // If user defined a custom method to create routes
       templateVars.router.routes = this.options.build.createRoutes(
diff --git a/packages/config/src/config/router.js b/packages/config/src/config/router.js
index 200c22bfc..878920675 100644
--- a/packages/config/src/config/router.js
+++ b/packages/config/src/config/router.js
@@ -12,5 +12,6 @@ export default () => ({
   parseQuery: false,
   stringifyQuery: false,
   fallback: false,
-  prefetchLinks: true
+  prefetchLinks: true,
+  trailingSlash: undefined
 })
diff --git a/packages/utils/src/route.js b/packages/utils/src/route.js
index 5beeeadd2..a55ad5be5 100644
--- a/packages/utils/src/route.js
+++ b/packages/utils/src/route.js
@@ -136,7 +136,8 @@ export const createRoutes = function createRoutes ({
   srcDir,
   pagesDir = '',
   routeNameSplitter = '-',
-  supportedExtensions = ['vue', 'js']
+  supportedExtensions = ['vue', 'js'],
+  trailingSlash
 }) {
   const routes = []
   files.forEach((file) => {
@@ -173,6 +174,11 @@ export const createRoutes = function createRoutes ({
         }
       }
     })
+    if (trailingSlash !== undefined) {
+      route.pathToRegexpOptions = { ...route.pathToRegexpOptions, strict: true }
+      route.path = route.path.replace(/\/+$/, '') + (trailingSlash ? '/' : '')
+    }
+
     parent.push(route)
   })
 


Test patch:
diff --git a/packages/config/test/__snapshots__/options.test.js.snap b/packages/config/test/__snapshots__/options.test.js.snap
index e86ab8530..df8249045 100644
--- a/packages/config/test/__snapshots__/options.test.js.snap
+++ b/packages/config/test/__snapshots__/options.test.js.snap
@@ -322,6 +322,7 @@ Object {
     "routes": Array [],
     "scrollBehavior": null,
     "stringifyQuery": false,
+    "trailingSlash": undefined,
   },
   "server": Object {
     "host": "localhost",
diff --git a/packages/config/test/config/__snapshots__/index.test.js.snap b/packages/config/test/config/__snapshots__/index.test.js.snap
index f6870e019..6fd7695c6 100644
--- a/packages/config/test/config/__snapshots__/index.test.js.snap
+++ b/packages/config/test/config/__snapshots__/index.test.js.snap
@@ -289,6 +289,7 @@ Object {
     "routes": Array [],
     "scrollBehavior": null,
     "stringifyQuery": false,
+    "trailingSlash": undefined,
   },
   "server": Object {
     "host": "localhost",
@@ -618,6 +619,7 @@ Object {
     "routes": Array [],
     "scrollBehavior": null,
     "stringifyQuery": false,
+    "trailingSlash": undefined,
   },
   "server": Object {
     "host": "localhost",
diff --git a/packages/utils/test/__snapshots__/route.test.js.snap b/packages/utils/test/__snapshots__/route.test.js.snap
index 7ac1ccbc0..f65199dd8 100644
--- a/packages/utils/test/__snapshots__/route.test.js.snap
+++ b/packages/utils/test/__snapshots__/route.test.js.snap
@@ -119,3 +119,178 @@ Array [
   },
 ]
 `;
+
+exports[`util: route util: route create createRoutes should enforce trailing slashes when configured to 1`] = `
+Array [
+  Object {
+    "chunkName": "pages/parent/index",
+    "component": "/some/nuxt/app/pages/parent/index.vue",
+    "name": "parent",
+    "path": "/parent/",
+    "pathToRegexpOptions": Object {
+      "strict": true,
+    },
+  },
+  Object {
+    "chunkName": "pages/snake_case_route",
+    "component": "/some/nuxt/app/pages/snake_case_route.vue",
+    "name": "snake_case_route",
+    "path": "/snake_case_route/",
+    "pathToRegexpOptions": Object {
+      "strict": true,
+    },
+  },
+  Object {
+    "chunkName": "pages/parent/child/index",
+    "component": "/some/nuxt/app/pages/parent/child/index.vue",
+    "name": "parent-child",
+    "path": "/parent/child/",
+    "pathToRegexpOptions": Object {
+      "strict": true,
+    },
+  },
+  Object {
+    "chunkName": "pages/parent/child/test",
+    "component": "/some/nuxt/app/pages/parent/child/test.vue",
+    "name": "parent-child-test",
+    "path": "/parent/child/test/",
+    "pathToRegexpOptions": Object {
+      "strict": true,
+    },
+  },
+  Object {
+    "children": Array [
+      Object {
+        "chunkName": "pages/another_route/_id",
+        "component": "/some/nuxt/app/pages/another_route/_id.vue",
+        "name": "another_route-id",
+        "path": "",
+        "pathToRegexpOptions": Object {
+          "strict": true,
+        },
+      },
+    ],
+    "chunkName": "pages/another_route/_id",
+    "component": "/some/nuxt/app/pages/another_route/_id.vue",
+    "name": "another_route-id",
+    "path": "/another_route/:id?/",
+    "pathToRegexpOptions": Object {
+      "strict": true,
+    },
+  },
+  Object {
+    "chunkName": "pages/subpage/_param",
+    "component": "/some/nuxt/app/pages/subpage/_param.vue",
+    "name": "subpage-param",
+    "path": "/subpage/:param?/",
+    "pathToRegexpOptions": Object {
+      "strict": true,
+    },
+  },
+  Object {
+    "chunkName": "pages/index",
+    "component": "/some/nuxt/app/pages/index.vue",
+    "name": "index",
+    "path": "/",
+    "pathToRegexpOptions": Object {
+      "strict": true,
+    },
+  },
+  Object {
+    "chunkName": "pages/_param",
+    "component": "/some/nuxt/app/pages/_param.vue",
+    "name": "param",
+    "path": "/:param/",
+    "pathToRegexpOptions": Object {
+      "strict": true,
+    },
+  },
+]
+`;
+
+exports[`util: route util: route create createRoutes should remove trailing slashes when configured to 1`] = `
+Array [
+  Object {
+    "chunkName": "pages/index",
+    "component": "/some/nuxt/app/pages/index.vue",
+    "name": "index",
+    "path": "",
+    "pathToRegexpOptions": Object {
+      "strict": true,
+    },
+  },
+  Object {
+    "chunkName": "pages/parent/index",
+    "component": "/some/nuxt/app/pages/parent/index.vue",
+    "name": "parent",
+    "path": "/parent",
+    "pathToRegexpOptions": Object {
+      "strict": true,
+    },
+  },
+  Object {
+    "chunkName": "pages/snake_case_route",
+    "component": "/some/nuxt/app/pages/snake_case_route.vue",
+    "name": "snake_case_route",
+    "path": "/snake_case_route",
+    "pathToRegexpOptions": Object {
+      "strict": true,
+    },
+  },
+  Object {
+    "chunkName": "pages/parent/child/index",
+    "component": "/some/nuxt/app/pages/parent/child/index.vue",
+    "name": "parent-child",
+    "path": "/parent/child",
+    "pathToRegexpOptions": Object {
+      "strict": true,
+    },
+  },
+  Object {
+    "chunkName": "pages/parent/child/test",
+    "component": "/some/nuxt/app/pages/parent/child/test.vue",
+    "name": "parent-child-test",
+    "path": "/parent/child/test",
+    "pathToRegexpOptions": Object {
+      "strict": true,
+    },
+  },
+  Object {
+    "children": Array [
+      Object {
+        "chunkName": "pages/another_route/_id",
+        "component": "/some/nuxt/app/pages/another_route/_id.vue",
+        "name": "another_route-id",
+        "path": "",
+        "pathToRegexpOptions": Object {
+          "strict": true,
+        },
+      },
+    ],
+    "chunkName": "pages/another_route/_id",
+    "component": "/some/nuxt/app/pages/another_route/_id.vue",
+    "path": "/another_route/:id?",
+    "pathToRegexpOptions": Object {
+      "strict": true,
+    },
+  },
+  Object {
+    "chunkName": "pages/subpage/_param",
+    "component": "/some/nuxt/app/pages/subpage/_param.vue",
+    "name": "subpage-param",
+    "path": "/subpage/:param?",
+    "pathToRegexpOptions": Object {
+      "strict": true,
+    },
+  },
+  Object {
+    "chunkName": "pages/_param",
+    "component": "/some/nuxt/app/pages/_param.vue",
+    "name": "param",
+    "path": "/:param",
+    "pathToRegexpOptions": Object {
+      "strict": true,
+    },
+  },
+]
+`;
diff --git a/packages/utils/test/route.test.js b/packages/utils/test/route.test.js
index 4b66688b2..ee2e58eda 100644
--- a/packages/utils/test/route.test.js
+++ b/packages/utils/test/route.test.js
@@ -194,5 +194,15 @@ describe('util: route', () => {
       const routesResult = createRoutes({ files, srcDir, pagesDir })
       expect(routesResult).toMatchSnapshot()
     })
+
+    test.posix('createRoutes should enforce trailing slashes when configured to', () => {
+      const routesResult = createRoutes({ files, srcDir, pagesDir, trailingSlash: true })
+      expect(routesResult).toMatchSnapshot()
+    })
+
+    test.posix('createRoutes should remove trailing slashes when configured to', () => {
+      const routesResult = createRoutes({ files, srcDir, pagesDir, trailingSlash: false })
+      expect(routesResult).toMatchSnapshot()
+    })
   })
 })