Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect lowercasing of params for nested routes #140

Open
pechitook opened this issue Feb 9, 2023 · 2 comments
Open

Incorrect lowercasing of params for nested routes #140

pechitook opened this issue Feb 9, 2023 · 2 comments

Comments

@pechitook
Copy link

pechitook commented Feb 9, 2023

馃憢 Hello @jorgegorka

We found an issue which seems to be affecting only nested routes whose first path is a named param. For example

  {
    name: '/admin',
    onlyIf: { guard: userIsAdminGuard, redirect: '/login' },
    nestedRoutes: [
      {
        name: 'entity',
        nestedRoutes: [
          { name: 'index', component: Admin },
          { name: ':entityId/details', component: Details },
        ],
      },
    ],
  },

When trying to get /admin/entity/s0M3-iD/details the router would incorrectly case that to /admin/entity/s0m3-id/details which then fails to get the entity from the server due to case sensitive ids being used in the database.

By looking at the source code, it seems that this function could be the root cause. In a nested route context, the first part of the pathname could indeed be the named param.

From a naive perspective it seems unnecessary that a router library would make assumptions on the URL casing in any context, and just removing that .toLowerCase makes sense conceptually. Other approach could be to understand if this is a nested route and catch that separately.

Any thoughts on this? Happy to see this through into a PR once we land on a solution path.

Thanks :)

@pechitook
Copy link
Author

pechitook commented Feb 9, 2023

We think there are two valid workarounds

  1. Avoid using a named param as a first param in a nested route
  2. Avoid using nested routes at all, and declare each route independently

@flipkickmedia
Copy link

Ill try and fix this in the fork at https://github.com/flipkickmedia/svelte-router-spa/

PRs welcome if we can also have some tests written as well.

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

No branches or pull requests

2 participants