-
-
Notifications
You must be signed in to change notification settings - Fork 8.5k
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: Allow modifying docs
url prefix
#914
Conversation
Deploy preview for docusaurus-preview ready! Built with commit 59b95d7 |
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.
This is an AWESOME feature 😄
Just a small nit regarding the default route logic. @endiliey does this look good to you?
lib/core/defaults.js
Outdated
*/ | ||
|
||
module.exports = { | ||
DOCS_ROUTE: 'docs', |
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.
Please add this to lib/core/constants
instead.
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'll do it ASAP, thanks for the response.
lib/server/sitemap.js
Outdated
@@ -16,6 +16,10 @@ const utils = require('../core/utils'); | |||
|
|||
const siteConfig = require(`${CWD}/siteConfig.js`); | |||
|
|||
const {DOCS_ROUTE} = require('../core/defaults'); | |||
|
|||
siteConfig.docsRoute = siteConfig.docsRoute || DOCS_ROUTE; |
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.
We're repeating this logic a few times in the code. I think we could out this in lib/server/routing.js
, as a getDocsRoute
function. @endiliey WYDT?
@@ -97,6 +99,8 @@ customDocsPath: 'website-docs'; | |||
|
|||
`disableTitleTagline` - An option to disable showing the tagline in the title of main pages. Exclude this field to keep page titles as `Title • Tagline`. Set to `true` to make page titles just `Title`. | |||
|
|||
`docsRoute` - docsRoute for your site. For example, `docs` is the docsRoute of https://docusaurus.io/docs/en/installation/ |
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.
Change to - Route of the documentation. By default, /docs
is the default route.
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.
Sorry for the delayed response.
Please rebase and change it to docsUrl
instead of docsRoute
at siteConfig for consistency with v2. Will review and merge after that
I'll do it ASAP |
siteConfig.js has a new mandatory field named *docsRoute* which default value is 'docs' and that can be customized by the user. This change will allow users who uses the library to host guides and tutorials to customize their websites by assign 'docsRoute' values like 'tutorials' or 'guides'. Fixes #879
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.
Sorry, I'm kinda new to this. Can you please help me trying to figure out what went wrong? |
We moved v1 files to |
Ok, so I need to redo the rebase but trying to match the folder. |
You should try deleting the |
Pretty sure we'd like something like this for Redux / React-Redux . The existing Redux docs URLs look like https://redux.js.org/recipes/configuringyourstore , with no |
Hi here, this is a nice feature to have! Is there anything we can do to help this PR to land? 🌝 |
I'm waiting a feedback from @endiliey |
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 bad. Seems that I'm the blocker here :|
I myself has a lot of hesitation on the PR due to v1 architecture that makes the whole implementation prone to breaks. Just look at the code and number of opaque calls like
const {getDocsUrl} = require('../../../lib/server/routing')
It's not really your fault, it just because the architecture had lot of opaque calls like require(process.cwd()/siteConfig.js
In addition, all the user will have to manually edit their url on website/pages/**.js
and website/core/Footer.js
since any changes we made on Docusaurus won't be ported to userland (unless it's a first install)
Furthermore, we have long implemented this in v2 and ofc the implementation is cleaner because we properly load the siteConfig (we even allow siteConfig.js live reload unlike v1) and pass it as props to whole React component of the app through React Context API.
WDYT @yangshun
Thanks for your feedback @endiliey, I'll look at it in-depth ASAP. I know you must be busy so thanks again for having found out time to reply. |
Since I'm busy with school and work, I'd like if someone could look at it, too — @wgao19 |
Hey @domcorvasce yea I'd like to help. Let me pull up a PR to your branch soon. @endiliey, regarding your comment here:
We do have hardcoded Meanwhile, may I ask when will v2 release? |
This ensures correct routing for customized `baseUrl` and `docsUrl`. - Changed all routing functions to take `siteConfig` instead of `siteConfig.baseUrl` - Updated tests accordingly
WiP: Fix Edge Cases for Customized `docsUrl`
How's this PR looking? This is the main thing holding us back from using it for the main Redux docs site, because I'd like to have it natively generate URLs that are the same as what we have right now with Gitbook (such as https://redux.js.org/introduction/coreconcepts ). |
We will merge this sometimes over the weekend. I'm trying to find some time myself to actually edit/patch the PR and test it out. Edit: Edit 2: |
Custom permalinks would be another viable option, sure. |
@endiliey Thanks so much for the work! |
We also want to change the documentation updates in this PR to be applied to the master docs instead of version 1.3.3 |
docs
url prefix
Alright, I managed to patch this PR and test it out in a fresh project, seems working fine so far. One significant change that I made is that now So now everytime user needs to access const siteConfig = require(`${process.cwd()}/siteConfig.js`);
const docsUrl = siteConfig.docsUrl || 'docs'; At the end of the day, all user can now access the siteConfig in their React component like this const React = require('react');
class MyPage extends React.Component {
render() {
const {config} = this.props;
return <div>{config.title}</div>;
}
}
module.exports = MyPage; Hmm anyway, P.S If anything breaks, we'll fix it in separate PR Thank you @domcorvasce for starting this and @wgao19 on helping out |
Just used this on the React-Redux docs at https://react-redux.js.org . Thanks! |
Good day, Does anyone know how to rename the Thank you! |
@yangshun Thank you very much it works great! For reference, added a
|
siteConfig.js has a new optional field named docsUrl which default value is 'docs'.
This change will allow users who uses the library to host guides and tutorials to customize their websites by assign 'docsUrl' values like 'tutorials' or 'guides'.
Fixes #879
Motivation
See #879 - I think this is a great addition to the project.
Have you read the Contributing Guidelines on pull requests?
Yep
Test Plan
It uses the existing tests to be sure everything's working fine.
Related PRs
--