Skip to content

[🐞] Bug when using rewriteRoutes with 3+ routes and trailingSlash set to false #5247

Open
@kevintakeda

Description

@kevintakeda

Which component is affected?

Qwik City (routing)

Describe the bug

The bug

A simple rewriteRoutes for i18n like this seems to be creating an incorrect regex.
Crashes with the error: SyntaxError: Invalid regular expression: /^\/fr\/it\/es\/test-es\/: \ at end of pattern

const trailingSlash = false;
const rewriteRoutes = [
  {
    prefix: 'es',
    paths: {
      '': '',
      test: 'test-es',
    },
  },
  {
    prefix: 'it',
    paths: {
      '': '',
      test: 'test-it',
    },
  },
  {
    prefix: 'fr',
    paths: {
      '': '',
      test: 'test-fr',
    },
  },
];

But, 2 or less routes works fine.
Or, if I switch the rewrite paths specifically to '/' : '' and set trailingSlash to true, then everything works as expected.
However, if I set trailingSlash to false, it doesn't crash, but it also doesn't match the routes.

Inconsistency when using grouped layout and one route.

Seems that '/': '' configuration doesn't match index routes such as /fr when trailingSlash is set to false , but it matches when not using grouped layout as shown in the reproduction code.

Possibly unintended

Multiples slashes doesn't redirect or give an "404 error", I assume is not intended to allow paths like:
[OK 200] example.com/path
[OK 200] example.com//path
[OK 200] example.com///path
[OK 200] example.com////path

Another regex problem

Rewriting to the same path also produces an incorrect regex.

// 3. Also crashes
const trailingSlash = true;
const rewriteRoutes = [
  {
    prefix: 'es',
    paths: {
      test: 'test',
    },
  },
  {
    prefix: 'it',
    paths: {
      test: 'test',
    },
  },
];

Reproduction

https://stackblitz.com/edit/qwik-starter-auqqff?file=vite.config.ts

Steps to reproduce

  1. Run npm install && npm run dev
  2. It should give an error
  3. Play with vite.config.js, everything else is just boilerplate.
  4. Try removing a single route and check if works

System Info

"vite": "4.4.9"
"@builder.io/qwik": "^1.2.12"
"@builder.io/qwik-city": "^1.2.12"

Additional Information

The main issue with rewriteRoutes is that it doesn't offer the flexibility to rewrite only specific paths. Right now, you're required to rewrite all potential paths, and you can't rewrite them to the same path.

For example, if someone wants to use the prefix feature to match any route but with a specific prefix, they'll have to rewrite all the paths, which can be quite limiting.

I believe it would be a nice feature if rewriteRoutes could automatically rewrite routes to the default route in specific situations. For instance, if a request arrives with a prefix like /it, and the prefix matches, but the path / isn't explicitly defined as a key in the paths. In this case, it should automatically rewrite it to the default path without the prefix, only if / itself isn't specified as a value in the paths object. This way, it can safely fallback as if there's a rule to rewrite /it to /.

Metadata

Metadata

Assignees

No one assigned

    Labels

    COMP: qwik-cityTYPE: bugSomething isn't workingWAITING FOR: teamWaiting for one of the core team members to review and reply

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions