Vue3: Fix warnings with vue-router

[aalves08]

Setup

• Rancher version: 2.10 - Vue3
• Rancher UI Extensions:
• Browser type & version:

Describe the bug

There's a lot of warnings from vue-router due to the fact that we use custom params in our application. According to https://github.com/vuejs/router/blob/main/packages/router/CHANGELOG.md#414-2022-08-22 it's an anti-pattern that is discouraged to use, since we shouldn't define params that aren't defined on the route.

One of the biggest problems is that extensions rely on it, despite "hardcoding" it on their routes, so that the routes become unique and don't get a match with our broad wildcarded routes like:

broad dashboard routes /:product/c/:cluster

vs

elemental example /elemental/c/:cluster

With that in mind, since our architecture needs the product param to work it's magic, in an extension we get a lot of warnings. I believe that dashboard also throws a few warnings as well.

To Reproduce

• Run dashboard locally, vue3 edition, and check console

Result Registering/navigating to a route with these "anti-pattern" params throws warnings to the console

Expected Result

No warnings should be thrown to the console

Screenshots

Additional context

[cnotv]

https://github.com/vuejs/router/blob/main/packages/router/CHANGELOG.md#414-2022-08-22 has been advised against for years as it's an anti-pattern in routing for many reasons, one of them being reloading the page lose the params

This is so true; we lose parameters on many pages and it is not possible to access them from the URL or reload. It may be worth to suppress the warning (if possible) or either fix the issue.

[cnotv]

As far as I can see there's no way to disable the warnings unless we override the error handling and we start to hack the way out for all our cases. This would also require a plan to fix the issue, as it will probably be enforced or broken with updates.

@codyrancher also mentioned that this issue has been brought by a version bump, so we may even restore the previous version instead of hacking.

[aalves08]

@cnotv My opinion is that either we fix this properly which, worst case scenario, might implicate the change of all route definitions in Dashboard and extensions OR we pin it to an older version and assume the tech debt for the future.

One thing that would be heartbreaking is for us to deal with this in another version and we would have yet another breaking change for extensions in a different version when it could have been dealt with along with other breaking changes.

[cnotv]

We will have for sure more breaking changes in the future, next to the list Vite. This one also does not seem to break yet.

It seems to me that there are several operations to do the refactoring:

• List known warnings
• Identify methods and functions triggering the warning, e.g. based on console report:
  • router.resolve()
  • router.replace()
  • fetchClusterResources(this.$store, METRIC.NODE, { force: true } ); (no idea why log points here)
  • this.showSearchModal = true; (no idea why log points here)
• Search through the code every instance where we do that, based on console report:
  • shell/components/nav/Type.vue
  • shell/components/nav/Group.vue
  • shell/pages/c/_cluster/explorer/index.vue
  • shell/components/nav/Header.vue
• Identify all the undefined (or artificial as they say) parameters, so far just these 2:
  • resource
  • product
  • I have not seen others
• List down all the definitions for each parameter or replace them with data binding as they suggest, although I doubt this last one

These are the occurrences in the router where are not defined:

I do not see the issue happening for others such as :namespace or :cluster.

Log

We could try a quick fix for these 2 parameters, although I rather recommend a single issue with proper investigation. This whole process is way too much for such short notice and for fitting our release planning, so I would rather opt for a version rollback.

[richard-cox]

I've doubled checked and the warnings do not appear in production builds. That was my biggest concern, as it confuses users and makes support tasks harder.

If there are no functional issues then I'd vote we move this out to 2.11. I don't think there are, @cnotv and @aalves08 can you confirm?

[cnotv]

I like my console log to be white and empty 😄, but agree to be for 2.11 at this point.

[aalves08]

There are no functional issue that I am aware of, but we haven't investigated if a proper fix would introduce breaking changes on extensions and that's what I am trying to avoid in the near future since we got them on 2.9 and 2.10 and that raises concerns for me.

We can leave this out for now but if the proper fix introduces breaking changes then I'd say that the milestone needs to be adjusted from 2.11 to another version where we aggregate more breaking changes 🙏

[cnotv]

If I got it right, in the current state, you are just not able to retrieve the parameter from the router, but I did not validate the hypothesis. In such case I doubt you need to know what resource or product is or at least not from the router.

[codyrancher]

@aalves08 a fix shouldn't impact extensions. Routes don't need to change to resolve this, we just need to stop resolving routes with extraneous params as @cnotv pointed out here https://github.com/rancher/dashboard/issues/12128#issuecomment-2401708209.

In fact most of the warnings I saw in both dashboard and harvester are resolved with this experimental change I made (more care needed) https://github.com/rancher/dashboard/pull/12193

As @richard-cox pointed out the warning is wrapped to ensure it doesn't make it into production

I'm going to bump this to 2.11. Feel free to bump it back if you think it needs to be fixed sooner.