single-spa-layout icon indicating copy to clipboard operation
single-spa-layout copied to clipboard

No way to stop matching after matching route was already found

Open ronald-d-rogers opened this issue 3 years ago • 7 comments

We're having an issue where multiple routes are being matched under a single route when we only want the first matched.

Related: https://github.com/single-spa/single-spa/issues/1001

To use the tests as an example:

{
  mode: "history",
    base: "/",
    containerEl: "body",
    routes: [
      { type: "application", name: "nav" },
      {
        type: "route",
        path: "app1",
        routes: [
          { type: "application", name: "app1" },
          {
            type: "route",
            path: "subroute",
            routes: [{ type: "application", name: "subroute" }],
          },
        ],
      },
      {
        type: "route",
        path: "app2",
        routes: [
          { type: "application", name: "app2" },
          {
            type: "route",
            path: "subroute",
            routes: [{ type: "application", name: "subroute" }],
          },
        ],
      },
+     {
+       type: "route",
+       path: "app3",
+       match: "first",
+       routes: [
+         { type: "application", name: "app3" },
+         {
+           type: "route",
+           path: "search",
+           break: true,
+           routes: [{ type: "application", name: "search" }],
+         },
+         {
+           type: "route",
+           path: "/:id",
+           routes: [{ type: "application", name: "detail" }],
+         }
+       ],
+     },
      {
        type: "route",
        path: "users/:id",
        routes: [
          { type: "application", name: "user-home" },
          {
            type: "route",
            path: "settings",
            routes: [{ type: "application", name: "user-settings" }],
          },
        ],
      },
      {
        type: "route",
        default: true,
        routes: [{ type: "application", name: "not-found" }],
      },
      { type: "application", name: "footer" },
    ],
  }
}

With the addition of the route above, when the user navigates to /app3/search, we want only the search route to match.

To be specific, I want this:

it(`matches single routes`, () => {
  expect(matchRoute(routesConfig, "/app3/search")).toMatchObject({
    ...routesConfig,
    routes: [
      { type: "application", name: "nav" },
      {
        type: "route",
        path: "app3",
        routes: [
          { type: "application", name: "app3" },
          {
            type: "route",
            path: "search",
            break: true,
            routes: [{ type: "application", name: "search" }],
          },
        ],
      },
      { type: "application", name: "footer" },
    ],
  });
});

But without this update I get:

[
  { type: "application", name: "nav" },
  {
    type: "route",
    path: "app3",
    routes: [
      { type: "application", name: "app3" },
      {
        type: "route",
        path: "search",
        break: true,
        routes: [{ type: "application", name: "search" }],
      },
    ],
  },
  { type: "application", name: "footer" },
]

But without this update I get:

[
  { type: "application", name: "nav" },
  {
    type: "route",
    path: "app3",
    routes: [
      { type: "application", name: "app3" },
      {
        type: "route",
        path: "search",
        break: true,
        routes: [{ type: "application", name: "search" }],
      },
+     {
+       type: "route",
+       path: "/:id",
+       routes: [{ type: "application", name: "detail" }],
+     },
    ],
  },
  { type: "application", name: "footer" },
]

And there doesn't appear to be anything that can be done to get this combining any of the currently available routing layout/options.

ronald-d-rogers avatar Aug 03 '22 15:08 ronald-d-rogers

So there appears to be multiple ways to solve for this, which I have working.

It is a question of what is the better API I think for me at this point...

Assuming you agree with this change, here are some potential APIs.

Short-circuiting from the parent route:

<route match="first" path='app3'>
  <application ...>
  <route path='search'>...</route>
  <route path='/:id'>...</route>
</route>

or

Short-circuiting on the matched route:

<route path='app3'>
  <application ...>
  <route path='search' break>...</route>
  <route path='/:id'>...</route>
</route>

ronald-d-rogers avatar Aug 03 '22 16:08 ronald-d-rogers

I think the following would work for you, can you confirm?

<route path="app3">
  <route path="search"><application name="search"></application></route>
  <route default="true"><application name="app3"></application></route>
</route>

internettrans avatar Aug 04 '22 21:08 internettrans

Will give it a try! Thanks.

ronald-d-rogers avatar Aug 04 '22 22:08 ronald-d-rogers

So I think the problem with this is we need the path to get the :id but both path and default cannot be truthy. The dev on point on this is ill at the moment. That however jives with the discussion in https://github.com/single-spa/single-spa/issues/1001.

The dev who was leading this initiative is sick with COVID ATM so no updates for a bit, but I will let you know how it goes!

ronald-d-rogers avatar Aug 06 '22 02:08 ronald-d-rogers

I went ahead and tested the suggestion made when you closed https://github.com/single-spa/single-spa/issues/1001 (https://github.com/single-spa/single-spa-layout/compare/main...ronald-d-rogers:test-single-spa-1001?expand=1) and all tests appear to be passing, so I don't know why this didn't work for us...

ronald-d-rogers avatar Aug 06 '22 04:08 ronald-d-rogers

The suggestion mentioned here: https://github.com/single-spa/single-spa/issues/1001#issuecomment-1147913510 will mount both apps in case of /search but will mount only mfe1 in case of anything else, ex: /potato

rami8k avatar Sep 06 '22 16:09 rami8k

I think the following would work for you, can you confirm?

<route path="app3">
  <route path="search"><application name="search"></application></route>
  <route default="true"><application name="app3"></application></route>
</route>

This would work if we have flat routing, our issue is with nested routing.

rami8k avatar Sep 06 '22 16:09 rami8k