-
-
Notifications
You must be signed in to change notification settings - Fork 8.7k
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
fix(v2):pass siteConfig as prop to pages #3025
Conversation
Deploy preview for docusaurus-2 ready! Built with commit 54a9437 |
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.
works, but not ideal :'(
const configPath = await createData( | ||
`${docuHash('config')}.json`, | ||
JSON.stringify({siteConfig: context.siteConfig}, null, 2), | ||
); |
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.
Unfortunately, this works but is not ideal. We should try to reuse the existing config file of the .docusaurus folder.
If you look at App.tsx it imports it directly: import siteConfig from '@generated/docusaurus.config';
We should avoid generating a new json file containing exactly the same data.
I don't know exactly how to do this for now, this may be more complex than I thought.
Some idea I have is to add a "@theme/internal/PageHandler" component, that would call useDocusaurusConfig
and inject it into the page.
Maybe there's a simpler option, but I don't know which one for now. If you want to explore an alternative solution that does not call createData() that would be nice ;)
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 had the same idea but then thought if docusaurus.config
file is a core dependency we shouldn't use it.
await Promise.all( | ||
content.map(async (metadataItem) => { | ||
const {permalink, source} = metadataItem; | ||
addRoute({ | ||
path: permalink, | ||
component: source, | ||
exact: true, | ||
modules: { | ||
config: configPath, | ||
}, |
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.
maybe the solution is just something like
modules: {
config: "@generated/docusaurus.config",
},
never tried this yet but that's worth giving it a shot ;)
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 changed to use generated file
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.
My first implementation was using docusaurus.config
but on second thoughts using a core file that is out of control of the plugin is also not ideal since docusaurus api doesn't specify these files will always be present for use.
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.
Actually I tried and it works fine with @generated/docusaurus.config
, as we have a webpack alias
I think it's simpler to just use this value instead of the file path so I'll update and merge this
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 ask Rohan to get u permission to push to our branches
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.
Thanks
didn't see you were all using the same fork :)
Normally it's a pre-checked checkbox on each PR (I think)
Co-authored-by: Sébastien Lorber <[email protected]>
Motivation
Migrating from v1 to v2 all the components are converted to Functional Component. It fixes #2925
Have you read the Contributing Guidelines on pull requests?
yes
Test Plan
I checked it manually props are passed perfectly