kriasoft / universal-router

A simple middleware-style router for isomorphic JavaScript web apps
https://www.kriasoft.com/universal-router/
MIT License
1.7k stars 105 forks source link

Bug: Empty children array shouldnt be required when dynamically defining routes #192

Closed joezappie closed 3 years ago

joezappie commented 3 years ago

I'm submitting a ...

Bug reports

I am dynamically defining my routes. Each module defines its own routes array, so when its added to the view I set the route.children object as suggested in issue #117.

I'm seeing an issue where the dynamic routes are not run unless I first add an empty children array to the route. Is there a reason (possibly optimization?) why if I don't add an empty children array, it then will not check for children.

Example:

const router = new UniversalRouter([
  {
    path: '/main',
    action: async ({next, route}) => {
        const view = new MainView();
        route.children = view.routes;
        return await next();
      }
    },
  }
]);

class MainView {
  constructor() {
    super();

    this.routes = [
      {
        path: '/test',
        action: () => {};
      }
    ];
  }
}

In this example, going to /main/test will not run. Changing the upper route to include an empty children object enables it to work:

const router = new UniversalRouter([
  {
    path: '/main',
    action: async ({next, route}) => {
        const view = new MainView();
        route.children = view.routes;
        return await next();
      }
    },
    children: []
  }
]);

After this modification going to /main/test properly is executed.

frenzzy commented 3 years ago

It is not a bug, it is an optimization technique to build regex per route only once: https://github.com/kriasoft/universal-router/blob/37f7376f67b9d254400bb77b661d7f888138a376/src/UniversalRouter.ts#L168-L171

joezappie commented 3 years ago

Gotcha, I will close since its not a bug. I'll just have to get used to not forgetting that as all my routes are dynamically generated.