Skip to content

Optional dynamic route not generated as expected #7571

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Fiv5 opened this issue Jun 20, 2020 · 5 comments · Fixed by #7843
Closed

Optional dynamic route not generated as expected #7571

Fiv5 opened this issue Jun 20, 2020 · 5 comments · Fixed by #7843

Comments

@Fiv5
Copy link

Fiv5 commented Jun 20, 2020

Version

v2.13.0

Reproduction link

https://codesandbox.io/s/naughty-framework-qb4jg?file=/pages/home/foo/edit/_id.vue

Steps to reproduce

see the repo link folder, same as:

├── about.vue
├── home
│   ├── _user.vue
│   ├── foo
│   │   ├── edit
│   │   │   └── _id.vue
│   │   ├── index.vue
│   │   └── list
│   │       └── index.vue
│   └── foo.vue
└── index.vue

What is expected ?

routes: [{
    path: "/about",
    component: _bde21ea8,
    name: "about"
  }, {
    path: "/home/foo",
    component: _aef39672,
    children: [{
      path: "",
      component: _ba586bec,
      name: "home-foo"
    }, {
      path: "list",
      component: _43d1f2ce,
      name: "home-foo-list"
    }, {
    //  expect the path is optional
      path: "edit/:id?",
      component: _53866cad,
      name: "home-foo-edit-id"
    }]
  }, {
    path: "/home/:user?",
    component: _324f446b,
    name: "home-user"
  }, {
    path: "/",
    component: _7e72691e,
    name: "index"
  }]

What is actually happening?

routes: [{
    path: "/about",
    component: _bde21ea8,
    name: "about"
  }, {
    path: "/home/foo",
    component: _aef39672,
    children: [{
      path: "",
      component: _ba586bec,
      name: "home-foo"
    }, {
      path: "list",
      component: _43d1f2ce,
      name: "home-foo-list"
    }, {
      //  but it is not optional router
      path: "edit/:id",
      component: _53866cad,
      name: "home-foo-edit-id"
    }]
  }, {
    path: "/home/:user?",
    component: _324f446b,
    name: "home-user"
  }, {
    path: "/",
    component: _7e72691e,
    name: "index"
  }]
This bug report is available on Nuxt community (#c10817)
@ghost ghost added the cmty:bug-report label Jun 20, 2020
@Fiv5
Copy link
Author

Fiv5 commented Jun 20, 2020

In this case, the /home/:user?is optional as expected, when I remove foo.vue, it become not optional.

@Fiv5
Copy link
Author

Fiv5 commented Jun 20, 2020

I tried to solve this problem myself and changed some code in cleanChildrenRoutes(packages/utils/src/route.js), which worked well locally. Just to give some references. : )

function cleanChildrenRoutes(
  routes: RouteConfig[],
  isChild = false,
  routeNameSplitter = '-',
): RouteConfig[] {
  // let start = -1
  const regExpIndex = new RegExp(`${routeNameSplitter}index$`)
  const routesIndex: any[] = []
  routes.forEach((route) => {
    if (regExpIndex.test(route.name!) || route.name === 'index') {
      // Save indexOf 'index' key in name
      const res = route.name!.split(routeNameSplitter)
      // const s = res.indexOf('index')
      // start = start === -1 || s < start ? s : start
      routesIndex.push(res)
    }
  })
  routes.forEach((route) => {
    route.path = isChild ? route.path.replace('/', '') : route.path
    if (route.path.includes('?')) {
      // const names = route.name!.split(routeNameSplitter)
      const paths = route.path.split('/')
      if (!isChild) {
        paths.shift()
      } // clean first / for parents
      
      /******************* here is my code ****************** */
      const dynamicIndex: number[] = []
      paths.forEach((p, index) => {
        if (p.startsWith(':') && p.endsWith('?')) dynamicIndex.push(index)
      })

      let n = routesIndex.length

      while (dynamicIndex.length && n--) {
        for (let i = dynamicIndex.length - 1; i >= 0; i--) {
          if (routesIndex[n].includes(paths[dynamicIndex[i]].slice(1, -1))) {
            paths[dynamicIndex[i]] = paths[dynamicIndex[i]].replace('?', '')
            dynamicIndex.pop()
          }
        }
      }
      /** ************************************************** */

      // routesIndex.forEach((r) => {
      //   const i = r.indexOf('index') - start //  children names
      //   if (i < paths.length) {
      //     for (let a = 0; a <= i; a++) {
      //       if (a === i) {
      //         paths[a] = paths[a].replace('?', '')
      //       }
      //       if (a < i && names[a] !== r[a]) {
      //         break
      //       }
      //     }
      //   }
      // })
      route.path = (isChild ? '' : '/') + paths.join('/')
    }
    route.name = route.name!.replace(regExpIndex, '')
    if (route.children) {
      if (route.children.find((child) => child.path === '')) {
        delete route.name
      }
      route.children = cleanChildrenRoutes(
        route.children,
        true,
        routeNameSplitter,
      )
    }
  })
  return routes
}

@stale
Copy link

stale bot commented Jul 20, 2020

Thanks for your contribution to Nuxt.js!
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
If you would like this issue to remain open:

  1. Verify that you can still reproduce the issue in the latest version of nuxt-edge
  2. Comment the steps to reproduce it

Issues that are labeled as pending will not be automatically marked as stale.

@stale stale bot added the stale label Jul 20, 2020
@aviramaz
Copy link

We're having the same issue on one of our projects. which worked fine before Nuxt upgrade to > 2.12.2.
Seems like some Nuxt core issue impacting existing project, Nuxt team, any directions on this one? @pi0 @atinux
Do you expect any quick fix on this one? or we should try to patch it? Thanks :)

@pi0
Copy link
Member

pi0 commented Aug 4, 2020

Should be fixed with v2.14.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants