-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
$site referencing to siteDataByRoute #159
Comments
You have very good points. And I too am very tempted to make Unlike They can still handle i18n logics using the So For example, another consistency issue I think we could have is, multi sidebar settings. When you're on Therefore, I think being "opt-in" to something is better than "opt-out". Where we have bare basic We should definitely change export default {
Layout,
NotFound: () => 'custom 404',
enhanceApp({ app, router, siteData, siteDataByRoute, pageData }) {
// ...
}
} P.S. |
I thought the same about $config, I even searched for $siteConfig to see if we could that name but it is being used for vitepress config. I understand your point and I agree that it is better to keep $site aligned with Vuepress, given that siteData in enhanceData is also pointing to the same thing. Maybe it is just a matter of finding a good name for $siteByRoute. Since $page is localized (and we will point users to But this is bikeshedding already, thanks for the detailed explanation again. And I think this issue is solved if enhanceData is coherent with $site 👍 |
Yeah I'm very open to discuss this. I think putting under Where else, site is a whole site, even though it is localized. It contains sidebar, navbar, which page do not care. Again, if we were calling the And while I was trying to come up with different names, I also felt like maybe it's going to be confusing to have almost identical stuff in a different name, or in different place. At least, I would like to change it to |
I have the same feeling about Agree with $page. I think I also thought about it because there are already configs at the $page level for how the site looks. For example: |
Very good point. Yeah the For now, I'll fix the inconsistency to close this issue. Let's think a bit more about the naming of |
Spawn from a discussion in #152
About siteDataByRoute, if $title and $description are localized, I think $site should also be localized pointing to siteDataByRoute instead of siteData. I think that users accessing $site from the markdown, should really be looking for $siteDataByRoute.
In enhanceApp siteData actually means siteDataByRoute: https://github.com/vuejs/vitepress/blob/master/src/client/app/index.ts#L118
So it makes sense for $site to follow suit.
We will be breaking compat slightly here with vuepress, but I think this is a good change that will help avoid i18n issues and it will be really confusing that siteData from enhanceApp is not the same as $siteData in vitepress.
If we do this change, we could expose unlocalized siteData with another name. Maybe $unlocalizedSite?
The text was updated successfully, but these errors were encountered: