-
Notifications
You must be signed in to change notification settings - Fork 918
feat(core): support meta in router #504
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
Conversation
|
Sounds like a good idea. In fact, the Thus, to go further, I think we can remove |
|
Got it, I will move it to default-theme in next commit. Do you think it's a good idea adding a new option in VuePress config for that limitation? |
meteorlxy
left a comment
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.
Node API docs should also be updated
|
I am not sure where should I put this description into, as I am not creating a new hook. And also the docs is not containing a detailed description about |
| // store title to display navbar and sidebar | ||
| extendsPageData: ({ filePathRelative, title }) => ({ | ||
| filePathRelative, | ||
| meta: { title }, |
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.
If extendsPageData is called multiple time, seems that the meta object will be replaced, instead of being merged?
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.
What about putting it into extendsPageOptions ? 🤔 (Just an idea, not fully considered)
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.
Nope, plugins may need to read user frontmatter then write something as page meta. Or in anothrt word, they probably need to know what the page has before generating some 'data'
This is the same as frontmatter, currently frontmatter could be possibly overided if users not speard the original ones.
Actually yesterday I was thing to add a new hook, warnings will be displayed if root property of meta was overided. But users may meet situation where they are trying to override some content. So I give up. LP
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.
I will speard the original meta to fix this in next commit
| /** | ||
| * Route meta data of the page | ||
| */ | ||
| meta?: Record<string, unknown> |
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.
Then meta would also be part of Page, so it should be added to the docs: [Node API > Page properties]
This PR is inspired by #228
For scalability considerations, VuePress v2 remove pages ( which used to contain page object with frontmatter, headers and so on). That's a greate improvement. But new issues are created with this changes. Now,
@vuepress/coreonly bind title infomation into router, so that we can only get title infomation about an exisiting route created by markdown. But this will get limitations under many situations.There are many reasons and situations that a plugin or a theme feature try to read pages meta in the client side.
Under current api designs, if a plugin and theme wants to do so, there are only two ways:
1. Makeing temp files
One must create temp files on the node side then load it on the client side, this sounds resonable, and this is exacly what @meteorlxy sugguested in #228, but once I built it, I found that this may also break scalability.
I am metting a lot of sitations once my plugin only want to read a short infomation about other pages (e.g.: createdtime, author) , but when I build temp files, I must repeated every route path in the file. And I also need to write functions to check possiable redirections, when I am wanting a certain path (e.g.: with
/a/b/index/a/b/and/a/b/index.htmlI have to convert to/a/b/).The temp file will look like this:
{ "/large/projects/long/path/a": "2021-01-01", "/large/projects/long/path/a": "2021-01-02", "/large/projects/long/path/c": "2021-01-03", ... }This is heavily breaking scalability in large projects because large sites with more pages offen have longer page urls. And if all plugins are doing the same way if they need it, the long path will be repeated serveral times.
So the final steps are:
extendPageDatahookLimitations:
extendPageDatahook won't be invoked during some data updates, so that no HMR.2. Using the unstable
usePagesData()This composition api is undocumented, and @meteorlxy also warns that he may move it. Even using this api, there are a lot of unconvinience.
router.resolveto get theroutefirst, the try to find the last matced route and check if it has redirections(if so do the front steps again), finally with the final route, one gets the pageKey fromroute.name. The code is complicated (around 50 lines)Limitations:
As each access is actually downloading the whole page js chunk,
pages.mapmeans downloading all page js files(Though gladly, it has HMR.)
I think for month to solve the above difficulties gracefully, and the only posiible solution I think out is this pr: allowing people to put meta in routes, and give the meta a size limitation.
What's the benifit of putting small meta infomation in route
There will still be good scalability, as page meta is limited to max 1kb size, (so that only 1MB size increasement with even 1024 page all with 1kb meta)
Plugins and themes should try to use a key in meta to store data they want. And they can directly get it through router synchronously, without: making tempfiles through
extendPageDatahook meanwhile repeating urls or getting the whole page data asynchronously through undocument api.This is exacly how vuepress is now storing page titles.