Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature request] Performance improvements with vue-router #1422

Closed
Mister-Hope opened this issue Oct 11, 2023 · 2 comments · Fixed by #1447
Closed

[Feature request] Performance improvements with vue-router #1422

Mister-Hope opened this issue Oct 11, 2023 · 2 comments · Fixed by #1447

Comments

@Mister-Hope
Copy link
Member

Mister-Hope commented Oct 11, 2023

Clear and concise description of the problem

Vue Router is not designed for tons of static path. Usually people should use dynamic routes when rendering lots of contents from database. Usually Vue Router won't need to map a looooong routes array to find a matching route. But in VuePress, all routes are static, and to provide redirects for /a/b and /a/b.md to /a/b.html, we are writing 3 routes for one page into vue-router.

The performance issue is from:

  • 1 markdown files generate 1 final record and 2 redirect record
  • vue-router support dynamic route in records, so it use expensive RegExp.test() againest all routes rather than simple string compare, and vue-router are "Mapping all routes" in each "route resolve"

Suggested solution

To simply solve this, we can change the array of static routes into a map, which is much faster.

We should:

  1. stop registering any static routes and maintain a page map
const pagesMap = new Map({
  '/a': {
     id: 'v-123456',
     // previously route meta
     data: {
      title: "A"
     }
  },
  '/b/': {
     id: 'v-123457',
     // previously route meta
     data: {
      title: "A"
     }
  },

  // ...
})
  1. use a catchAll (or fallback) route to render both static pages and 404 page.

    Since all pages are using VuePress component, we just need to modify the logic of getting page data:

const routes = [
      {
        name: 'vuepress-route',
        path: '/:catchAll(.*)',
        component: Vuepress,
      },
]
  router.beforeResolve(async (to, from) => {
    if (to.path !== from.path || from === START_LOCATION) {
      let pageId = '404'
      if(pageMap.has(to.path) {
        pageId = pageMap.get(to.path).id
      }

      if(to.path.endsWith('/index.html')) {
        // find if the one without suffix exists
        const redirectedPath = to.path.substring(0, to.path.length - 10);

        if(pageMap.has(redirectedPath) {
            pageId = pageMap.get(redirectedPath).id
        }
      }
        
      if(to.path.endsWith('.html')) {
        // find if the one without suffix exists
      }

      // also check the url encoded one and .md suffix

      // if no match at this point, then we should provide 404 page

      // write to meta into the record
      to.meta = pagesMap.get(pageId).meta;

      ;[to.meta._data] = await Promise.all([
        resolvers.resolvePageData(pageId),
        pagesComponents[to.name as string]?.__asyncLoader(),
      ])
    }
  })

In the above approach, we can even merge the original pagesComponents with it, as there won't 2 different pages using same path (an extra check and warnings can be done at node to check app.pages during prepare stage), changing the pagesComponents key from page hash key to page path can done the job:

const pagesMap = new Map({
  '/a': {
     content: () => import('./a.html.vue'),
     data: () => import('./a.html.js'),
     // previously route meta
     meta: {
      title: "A"
     }
  },
  '/b/': {
     content: () => import('./b.index.html.vue'),
     data: () => import('./b.index.html.js'),
     // previously route meta
     meta: {
      title: "A"
     }
  },

  // ...
 '404': {
   content: () => import('./404.html.vue'),
  }
})

  router.beforeResolve(async (to, from) => {
    if (to.path !== from.path || from === START_LOCATION) {
      let url = '404'
      if(pageMap.has(to.path) {
        pageComponent = to.path
      }

      if(to.path.endsWith('/index.html')) {
        // find if the one without suffix exists
        const redirectedPath = to.path.substring(0, to.path.length - 10);

        if(pageMap.has(redirectedPath) {
            url = redirectedPath 
        }
      }
        
      if(to.path.endsWith('.html')) {
        // find if the one without suffix exists
      }

      // also check the url encoded one and .md suffix

      // if no match at this point, then we should provide 404 page

      ;[to.meta._data] = await Promise.all([
        resolvers.resolvePageData(url),
        pageMap[url]?.__asyncLoader(),
      ])
    }
  })

That is, we just remove key usage at client, each page is identified by its final url.

Besides reducing the high cost with router.resolve(), we also minify route maps (with redirects) and page component maps into 1 single map. To archive furthermore, we can also merge pagesData map into it, so there is only 1 map holding vuepress client data.

Other tricky things can be done to reduce the output size, one is we can use shorter key with ts enums:

const enum PageMapKey {
  content: 'c',
  data: 'd',
  meta: 'm',
}

const pagesMap = new Map({
  '/a': {
     // will be compiled to 'c'
     [PageMapKey.content]: () => import('./a.html.vue'),
     [PageMapKey.data]: () => import('./a.html.js'),
     [PageMapKey.meta]: {
      title: "A"
     }
  },

 // ...
}
  1. Since the current behavior will change the result of router.getRoutes(), we need to expose the pageMap in @vuepress/client for developers and users to get page routes and contents.

Meanwhile, we can add built-in functions to cover normal usage:

import { pageMap, hasPage, getPageRoutes, resolve } from '@vuepress/client'
import { useRouter } from 'vue-router'

const pageRoutes = getPageRoutes();
// ['/', '/404.html', '/a.html', '/b/']

hasPage('/') // true
hasPage('/a') // true
hasPage('/a/') // false
hasPage('/b') // false
hasPage('/b/') // true

resolve('/a') // { path: '/a.html', data: { title: 'A' } }
resolve('/b/index.html') // { path: '/b/', data: { title: 'B' } }
resolve('/c') // { path: '/404.html', data: {} }

// navigation is still the same:
const goHome = () => {
 useRouter().push('/')
}

Alternative

No response

Additional context

No response

@Mister-Hope
Copy link
Member Author

@meteorlxy Could you check this idea?

@meteorlxy
Copy link
Member

Sounds good to me. Let's have a try

@Mister-Hope Mister-Hope linked a pull request Nov 20, 2023 that will close this issue
@Mister-Hope Mister-Hope linked a pull request Dec 5, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants