fix(router): show default child if trailingSlash is false

[manniL]

Resolves #6593

The bug was introduced by setting '/' instead of '' for child routes when router.trailingSlash is false:
https://github.com/nuxt/nuxt.js/blob/e39f54b44e203f6c47100f8fd779f34345bac772/packages/utils/src/route.js#L184.

Types of changes

• Bug fix (a non-breaking change which fixes an issue)
• New feature (a non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

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.

[manniL]

I kept the failing tests because I'm not sure about the impact of the introduced change and love to get feedback on the PR.

FAIL  test/unit/dynamic-routes.test.js
  ● dynamic routes › Check .nuxt/router.js

    expect(received).toEqual(expected) // deep equality

    - Expected
    + Received

      Array [
        "",
        "projects",
    -   "projects/:category",
    +   "projects/:category?",
        ":id",
        ":index/teub",
      ]

      49 |       // pages/test/users/*.vue
      50 |       expect(routes[4].children.length).toBe(5) // parent has 5 children
    > 51 |       expect(routes[4].children.map(r => r.path)).toEqual([
         |                                                   ^
      52 |         '',
      53 |         'projects',
      54 |         'projects/:category',

      at toEqual (test/unit/dynamic-routes.test.js:51:51)

[pimlie]

The failing test seems to be an issue, adding the ? to the route param will now also match the route when no param is given. In the test that might not be an issue because projects is already listed before, but if a user doesnt want to match /projects at all then this is a breaking change I guess?

[manniL]

My check for trailingSlash was wrong, thus the weird result. Should be fine with the latest commit.

[manniL]

Now only the missing route name is the difference which should be the right behavior.

[codecov-io]

Codecov Report

Merging #6594 into dev will increase coverage by 0.04%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##              dev    #6594      +/-   ##
==========================================
+ Coverage   62.69%   62.74%   +0.04%     
==========================================
  Files          84       84              
  Lines        3394     3398       +4     
  Branches      926      928       +2     
==========================================
+ Hits         2128     2132       +4     
  Misses       1018     1018              
  Partials      248      248              

Flag	Coverage Δ
#unittests	62.74% <100.00%> (+0.04%)	⬆️

Impacted Files	Coverage Δ
packages/utils/src/route.js	96.64% <100.00%> (+0.09%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c8e7397...65eca0c. Read the comment docs.

[manniL]

Ready for review ☺️

[pimlie]

Sorry for asking but why is deleting the name of the route the solution? It isnt something thats immediately obvious to me.

[manniL]

@pimlie Valid question! As reported in #6593, there will be a vue router warning otherwise advising to do so.

[vue-router] Named Route 'users' has a default child route. When navigating to this named route (:to="{name: 'users'"), the default child route will not be rendered. Remove the name from this route and use the name of the default child route for named links instead.
[vue-router] Duplicate named routes definition: { name: "users", path: "/users" }

[pimlie]

Thanks. Isnt deleting the route name then strictly speaking resolving a consequence of an issue? Ie why are the route names duplicate in the first place?

If it isnt solvable by fixing the duplicate route names, could we maybe add a comment explaining why we choose to fix it this way?

[manniL]

@pimlie Not sure why there were duped route names initially. Maybe @clarkdo could provide more info 🤔

[pimlie]

I think its because of this replace: https://github.com/nuxt/nuxt.js/blob/dev/packages/utils/src/route.js#L64

Because of that my-route and my-route/index will both be named my-route.

[RettuSh]

Hi! Is there any chance to fix this bug soon? Could I help you somehow?

[ems1985]

Hi, any updates on this?

Just FYI the issue also presents itself when router.trailingSlash is set to true

This is my folder structure:

Without the trailingSlash option my router.js routes looks like this:

routes: [{
    path: "/something",
    component: _2eb15426,
    children: [{
      path: "",
      component: _0e04f6b0,
      name: "something"
    }, {
      path: "childcomponent",
      component: _4484dbf3,
      name: "something-childcomponent"
    }]
  }, {
    path: "/",
    component: _d1bba076,
    name: "index"
  }],

With router.trailingSlash set to true

routes: [{
    path: "/something/",
    component: _2eb15426,
    pathToRegexpOptions: {"strict":true},
    name: "something",
    children: [{
      path: "childcomponent/",
      component: _4484dbf3,
      pathToRegexpOptions: {"strict":true},
      name: "something-childcomponent"
    }, {
      path: "",
      component: _0e04f6b0,
      pathToRegexpOptions: {"strict":true},
      name: "something"
    }]
  }, {
    path: "/",
    component: _d1bba076,
    pathToRegexpOptions: {"strict":true},
    name: "index"
  }]

Of course results in the following warnings:

 WARN  [vue-router] Named Route 'something' has a default child route. When navigating to this named route (:to="{name: 'something'"), the default child route will not be rendered. Remove the name from this route and use the name of the default child route for named links instead.


 WARN  [vue-router] Duplicate named routes definition: { name: "something", path: "/something/" }  

[codecov-commenter]

Codecov Report

Merging #6594 into dev will increase coverage by 0.03%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##              dev    #6594      +/-   ##
==========================================
+ Coverage   70.14%   70.17%   +0.03%     
==========================================
  Files          88       88              
  Lines        3758     3762       +4     
  Branches     1020     1022       +2     
==========================================
+ Hits         2636     2640       +4     
  Misses        912      912              
  Partials      210      210              

Flag	Coverage Δ
#unittests	70.17% <100.00%> (+0.03%)	⬆️

Impacted Files	Coverage Δ
packages/utils/src/route.js	96.66% <100.00%> (+0.09%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5272a48...cf93c3b. Read the comment docs.