-
Notifications
You must be signed in to change notification settings - Fork 920
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
feat(client): use visual routes #1433
Conversation
This pr reduce outputsize and improves performance at the same time. by avoiding:
Breaking changes
Other notesTo support clean url (e.g.: navigating to |
packages/client/src/router.ts
Outdated
// if no match at this point, then we should provide 404 page | ||
const pageInfo = pagesMap.get(pagePath) || pagesMap.get('/404.html')! | ||
|
||
// TODO: Added for backwards compatibility, remove in stable version | ||
to.meta = pageInfo.meta | ||
;[to.meta._data] = await Promise.all([ | ||
resolvers.resolvePageData(to.name as string), | ||
pagesComponents[to.name as string]?.__asyncLoader(), | ||
resolvers.resolvePageData(pageInfo, pagePath), | ||
pageInfo.comp?.__asyncLoader(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Notes:
Since we use beforeResolve to align pageData with routePath, thie means router.resolve
is still a heavy operation.
So we should avoid using router-link
as it internally calls router.resolve
to provide correct link on anchor tag. That is #1353 is still needed.
Though we no longer need RouterLink in that PR, VPLink should still be used by default in markdown and navbar, sidebar links.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@meteorlxy I prefer splitting the VPLink PR by updating the existing one #1353 after you merge this.
const convertedMdPath = path.endsWith('README.md') | ||
? path.substring(0, path.length - 9) | ||
: path.endsWith('.md') | ||
? path.substring(0, path.length - 3) + '.html' | ||
: path |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BTW, if we drop markdown link support, we can have higher performance here.
I would upvote to drop support for markdown links, since we already convert every markdown link in markdown files to <RouterLink>
(or <VPLink>
with VPLink pr).
At lease we can add an option here?
P.S: What do you think if we add a router
field in config files with options like :
router: {
cleanUrl: true,
resolveMarkdownLink: true,
// ...
}
No description provided.