vuejs / router

🚦 The official router for Vue.js
https://router.vuejs.org/
MIT License
3.75k stars 1.15k forks source link

feat: pluggable matcher #2166

Open skirtles-code opened 4 months ago

skirtles-code commented 4 months ago

Background

Original issue: #2132. In short, the performance of adding a large number of routes is slow, and is particularly problematic with SSR.

This PR explores the idea of a 'pluggable matcher'. This allows the matcher to be passed into the router, rather than being created internally. The potential benefits are twofold:

  1. In an SSR scenario, the matcher can be created upfront, rather than processing the individual routes for each request.
  2. Custom matchers can be used, potentially supporting different features or faster implementations.

The code presented here is a long way from being merged, but hopefully it'll work as a starting point to discuss how to proceed.

SSR usage

To get the benefits with SSR, we'd need to replace this:

const router = createRouter({
  history,
  routes: [/* ... */],
  strict,
  sensitive
})

with this:

// Do this once
const matcher = createDefaultMatcher({
  routes: [/* ... */],
  strict,
  sensitive
})

// Do this for each request
const router = createRouterWithMatcher({
  history,
  matcher: matcher.clone()
})

The clone() call creates a copy of the matcher, allowing for more routes to be added or removed without impacting the original matcher.

The performance of this is really good. Calling clone() is very fast and we skip all the overhead of processing the individual routes.

Custom matchers

This is trickier.

The usage is similar to the previous example, but with a custom implementation of matcher.

TypeScript isn't my strong suit, so perhaps this isn't as difficult as it seemed, but I found trying to get the types working as I wanted really difficult. The code in this PR is much less ambitious than some of my other attempts, but hopefully it'll still illustrate the difficulties.

For example, I tried writing a custom matcher that only supported the options path, name, component and components. You can see that at:

If you search through the code for Pain point, you'll find various points where I had to do things that didn't really make sense in the context of that matcher implementation. Some of those may be necessary for other reasons, but in many cases I was adding things just to appease TS. For example, the score and re properties are completely redundant, but the types force them to be included.

I did experiment with changing those types, adding generics all over the place, but it got a bit out of hand and I decided to aim a little lower for this initial PR. Instead, I chose to focus just on the route config.

As SimpleRouterMatcher only supports 4 options for the route, I wanted the types to reflect that. So SimpleRoute is a type that defines that format, with the matcher having type:

interface SimpleRouterMatcher extends GenericRouterMatcher<SimpleRoute> {}

You'll find RC used a generic in various places in the code. RC stands for 'route config'.

This then has an impact on the type of the addRoute() method of the router itself. New routes need to be added using the SimpleRoute format. e.g.:

const matcher = createSimpleRouterMatcher({
  routes: [
    {
      path: '/',
      component: HomeView
    }
  ]
})

const router = createRouterWithMatcher({ history, matcher })

// The type here is 'SimpleRoute', so options like 'strict' aren't available
router.addRoute({
  path: '/about',
  component: AboutView
})

This almost works. The major problem is $router and useRouter(), neither of which take account of the generic.

Other problems

The options property of the Router type is a bit fiddly. It needs to take account of the options passed to createRouterWithMatcher(), which aren't the same as createRouter().

Next steps

If we just want the performance benefits for SSR usage, we don't need support for custom matchers. That would dodge most of the type problems.

The custom matcher feature may tie into #2148. That PR is currently implemented as changes to the default matcher, but it could instead be implemented as a separate matcher implementation.

If the type problems can be overcome, I think we'd also need to consider adding helpers to make custom matchers easier to write. I found myself duplicating a lot of logic from the default matcher when I was writing SimpleRouterMatcher. If we do choose to pursue this further, I recommend anyone wanting to give feedback to attempt writing a custom matcher. I found going through that process quite enlightening.

Depending on what direction we choose to go in, we should also consider an RFC. At the very least we'd need some feedback from the SSR meta-frameworks about whether this is workable for them.

Relationship to other PRs

netlify[bot] commented 4 months ago

Deploy Preview for vue-router canceled.

Name Link
Latest commit be657dba3a7c81f8b30efbb4b4b23503e89cf805
Latest deploy log https://app.netlify.com/sites/vue-router/deploys/65e91f30f04dbf000835fa16
codecov-commenter commented 4 months ago

Codecov Report

Attention: Patch coverage is 83.33333% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 90.93%. Comparing base (089378b) to head (be657db). Report is 3 commits behind head on main.

Files Patch % Lines
packages/router/src/matcher/index.ts 83.33% 1 Missing :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #2166 +/- ## ========================================== - Coverage 91.01% 90.93% -0.09% ========================================== Files 24 24 Lines 1135 1136 +1 Branches 351 351 ========================================== Hits 1033 1033 - Misses 63 64 +1 Partials 39 39 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.