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

[Performance] Avoid using router-link #1352

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

[Performance] Avoid using router-link #1352

Mister-Hope opened this issue Jun 8, 2023 · 2 comments · Fixed by #1447
Assignees
Labels
enhancement New feature or request package:client

Comments

@Mister-Hope
Copy link
Member

Mister-Hope commented Jun 8, 2023

Clear and concise description of the problem

Currently vue-router has serious performance impact on sites with more than 500 pages.

See vue-router source code here: https://github.com/vuejs/router/blob/main/packages/router/src/RouterLink.ts

Each router-link called useLink inside it:

https://github.com/vuejs/router/blob/941b2131e80550009e5221d4db9f366b1fea3fd5/packages/router/src/RouterLink.ts#L211

And in useLink heavy calculations are running based on routes object at

https://github.com/vuejs/router/blob/941b2131e80550009e5221d4db9f366b1fea3fd5/packages/router/src/RouterLink.ts#L98-L138

just to solve the isActive and isExactActive.

These calculations are because of vue-router dynamic routes (routes can have params), and will likely to map all routes to find a matching one 6 times at least in production (even more in dev for devtools)

While all the links are rendered by router-link in pages, and all internal links in navbar and sitebar all based on router-link, which means we probably have a lot of router-link initialized or recalculated when opening and switching pages.

The are also 3 things to get even worse.

The first thing is that vue-router is not efficiency designed for large amount of routes. Usually people should use dynamic routes when rendering lots of contents from database, so VueRouter 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 second one is #1290. With this issue, when scrolling pages, plugin-active-hash will trigger the whole route object to update (including hash path fullPath params and others. So every router-link is recalculated to update state, with 18N (3 for similar routes of 1 page and 6 for use link) maps of all routes object. So on mobile devices, frames may lose when scrolling and people will feel stuck.

The third thing is the worse one: #1249

We are not using <router-view>, instead we are loading and mounting the page async on router.beforeResolve. This means the page is changed BEFORE route. Path is updated, so when rendering new page(and mounting its components), old route data is used and then a new route data needs to be recalculated at ones. We are supposed to focusing rerendering page layout components as fast as possible, while we did twice job here.

See https://docs.innenu.com/ as a reproduction, this website has NO additional components in page (plain HTML generated from markdown), while it has 1274 pages and nearly 4000 routes.

image

Suggested solution

  1. Fix [Bug report] Route path is not correct when navigating #1249
  2. Fix [Bug report] plugin-active-header-links trigger route.path changes #1290
  3. Avoid using <router-link, for static paths, all we need is to resolve the string link with router. resolve(link) and compare resolveRoute.path with currentRoute.path, only 1 mapping of routes should be done while calling router. Resolve,
    https://github.com/vuejs/router/blob/941b2131e80550009e5221d4db9f366b1fea3fd5/packages/router/src/RouterLink.ts#L98-L138

The above lines are not needed, so we should provide a <VPLink> in @vuepress/client and encourage to use this link when developing themes and plugins.

Also we should use a simple link with NO active state to render markdown anchor links:

import {
  type ComputedRef,
  type Ref,
  type SlotsType,
  type VNode,
  computed,
  defineComponent,
  h,
  toRef,
  unref,
} from "vue";
import {
  type NavigationFailure,
  type RouteLocation,
  useRoute,
  useRouter,
} from "vue-router";

const guardEvent = (event: MouseEvent): boolean | void => {
  // don't redirect with control keys
  if (event.metaKey || event.altKey || event.ctrlKey || event.shiftKey) return;
  // don't redirect when preventDefault called
  if (event.defaultPrevented) return;
  // don't redirect on right click
  if (event.button !== undefined && event.button !== 0) return;
  // don't redirect if `target="_blank"`
  if (event.currentTarget) {
    const target = (<HTMLElement>event.currentTarget).getAttribute("target");

    if (target?.match(/\b_blank\b/i)) return;
  }

  event.preventDefault();

  return true;
};

export interface LinkOptions {
  route: ComputedRef<RouteLocation & { href: string }>;
  href: Ref<string>;
  isActive: Ref<boolean>;
  navigate: (event?: MouseEvent) => Promise<void | NavigationFailure>;
}

export const useLink = (link: string | Ref<string>): LinkOptions => {
  const router = useRouter();
  const currentRoute = useRoute();

  const route = computed(() => router.resolve(unref(link)));

  const isActive = computed<boolean>(
    () => route.value.path === currentRoute.path
  );

  const navigate = (
    event: MouseEvent = {} as MouseEvent
  ): Promise<void | NavigationFailure> =>
    guardEvent(event) ? router.push(unref(link)).catch() : Promise.resolve();

  return {
    route,
    href: computed(() => route.value.href),
    isActive,
    navigate,
  };
};

export const MDLink= defineComponent({
  name: "MDLink",

  props: {
    /**
     * Link
     */
    to: {
      type: String,
      required: true,
    },
  },

  slots: Object as SlotsType<{
    default: () => VNode | VNode[];
  }>,

  setup(props, { slots }) {
    const router = useRouter();
    const to = toRef(props, "to");
    const route = computed(() => router.resolve(link));
    const navigate = (
      event: MouseEvent = {} as MouseEvent
    ): Promise<void | NavigationFailure> =>
      guardEvent(event) ? router.push(link).catch() : Promise.resolve();

    return (): VNode => {
      const children = slots.default && slots.default(linkOptions);

      return h(
        "a",
        {
          class: "vp-link",
          href: route.value.href,
          onClick: linkOptions.navigate,
        },
        children
      );
    };
  },
});

export const VPLink = defineComponent({
  name: "VPLink",

  props: {
    /**
     * Link
     */
    to: {
      type: String,
      required: true,
    },
  },

  slots: Object as SlotsType<{
    default: (linkOptions: LinkOptions) => VNode | VNode[];
  }>,

  setup(props, { slots }) {
    const to = toRef(props, "to");
    const linkOptions = useLink(to);

    return (): VNode => {
      const children = slots.default && slots.default(linkOptions);

      return h(
        "a",
        {
          class: ["vp-link", linkOptions.isActive.value ? "vp-active" : ""],
          href: linkOptions.href.value,
          onClick: linkOptions.navigate,
        },
        children
      );
    };
  },
});

The MDLink and VPLink do little work comparing with the original <router-link>, and if more aggressively, if we think that using the original link as anchor href is ok (at least I think so), we can even render the component with no cost:

export const PureLink = ({ to }, { slots }) => {
  const router = useRouter()
  const navigate = (
    event: MouseEvent = {} as MouseEvent
  ): Promise<void | NavigationFailure> =>
      guardEvent(event) ? router.push(link).catch() : Promise.resolve();

  return h('a', { class: 'vp-link', href: to, onClick: navigate }, slots.default?.());
}

Alternative

No response

Additional context

  • For large sites, we really need [Bug report] Route path is not correct when navigating #1249 to be fixed and a lighter routerComponent. The above website only takes <200ms time when switching pages on VitePress!
  • Since we are using single instance and trigger route path change and "await" the application render job done to do SSG work, this can also speed up SSG process greatly
@Mister-Hope Mister-Hope changed the title [Feature request] Avoid using vue-router [Feature request] Avoid using router-link Jun 8, 2023
@Mister-Hope Mister-Hope changed the title [Feature request] Avoid using router-link [Performance] Avoid using router-link Jun 8, 2023
@Mister-Hope Mister-Hope linked a pull request Jun 8, 2023 that will close this issue
@Mister-Hope
Copy link
Member Author

Mister-Hope commented Jun 8, 2023

Additional info:

image

With 6x slowdown, the expensive job of router.resolve can be easily seen. So though for small sites route meta is ok, for large sites, we may still need to avoid getting information from router as the behavior is mapping array.

https://github.com/vuepress/vuepress-next/blob/42c0bc2a352f85e0b4029e18a8a3270736e54229/ecosystem/theme-default/src/client/composables/useResolveRouteWithRedirect.ts#L8-L28

https://github.com/vuepress/vuepress-next/blob/42c0bc2a352f85e0b4029e18a8a3270736e54229/ecosystem/theme-default/src/client/composables/useNavLink.ts#L17-L26

@meteorlxy Any suggestions?

@Mister-Hope
Copy link
Member Author

@meteorlxy With #1360 #1353 #1357 #1361, performance on my theme docs (304 pages) has up to 500% on scrolling with hash changes and 70% in switching pages

@Mister-Hope Mister-Hope added enhancement New feature or request package:client and removed need review labels Dec 4, 2023
@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
Labels
enhancement New feature or request package:client
Projects
None yet
2 participants