-
-
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
Refactor(server): share logic between server.js & generate.js #856
Conversation
Deploy preview for docusaurus-preview ready! Built with commit ea03634 |
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! I'm glad we have ESLint in place before we started all these refactoring. Having linting makes things safer.
lib/server/docs.js
Outdated
function getRedirectStr(metadata) { | ||
if ( | ||
!env.translation.enabled || | ||
metadata.permalink.indexOf('docs/en') === -1 |
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.
Use .contains()
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.
Done.
I changed it to .includes()
instead. Couldn't find any .contains()
method in javascript 😂
lib/server/docs.js
Outdated
@@ -82,7 +83,7 @@ function replaceAssetsLink(oldContent) { | |||
return lines.join('\n'); | |||
} | |||
|
|||
function getComponent(rawContent, mdToHtml, metadata) { | |||
function getStr(rawContent, mdToHtml, metadata) { |
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 getMarkup
will be a better name
My bad @endiliey I got mixed up by the naming myself. I meant |
Changes
Mostly refactoring into reusable functions and add small test for it
Refactor duplicate/ similar logic in
server.js
andgenerate.js
andreadMetadata.js
togetMetadata
inblog.js
https://github.com/facebook/Docusaurus/blob/9f718a5097dfa7f549690a711fb151ea82a335a8/lib/server/generate.js#L156-L164
https://github.com/facebook/Docusaurus/blob/9f718a5097dfa7f549690a711fb151ea82a335a8/lib/server/server.js#L255-L267
https://github.com/facebook/Docusaurus/blob/9f718a5097dfa7f549690a711fb151ea82a335a8/lib/server/readMetadata.js#L326-L335
Refactor duplicate in
server.js
andgenerate.js
togetPages
inblog.js
https://github.com/facebook/Docusaurus/blob/9f718a5097dfa7f549690a711fb151ea82a335a8/lib/server/server.js#L202-L224
https://github.com/facebook/Docusaurus/blob/9f718a5097dfa7f549690a711fb151ea82a335a8/lib/server/generate.js#L180-L193
Refactor duplicate path replacement to a function in
blog.js
such ashttps://github.com/facebook/Docusaurus/blob/9f718a5097dfa7f549690a711fb151ea82a335a8/lib/server/readMetadata.js#L320-L325
https://github.com/facebook/Docusaurus/blob/9f718a5097dfa7f549690a711fb151ea82a335a8/lib/server/generate.js#L150-L155
Simplify docs redirectStr generation to function
getRedirectStr
in docs.jshttps://github.com/facebook/Docusaurus/blob/9f718a5097dfa7f549690a711fb151ea82a335a8/lib/server/generate.js#L95-L109
Refactor duplicate in
server.js
andgenerate.js
on blogPost generationhttps://github.com/facebook/Docusaurus/blob/9f718a5097dfa7f549690a711fb151ea82a335a8/lib/server/generate.js#L166-L175
https://github.com/facebook/Docusaurus/blob/9f718a5097dfa7f549690a711fb151ea82a335a8/lib/server/server.js#L269-L281
Add test for several key functions in new
blog.js
Remove redundant comments & some nits
Have you read the Contributing Guidelines on pull requests?
Yes
Test Plan
Links still work as per normal
https://deploy-preview-856--docusaurus-preview.netlify.com/
Related PRs
#847
#854