Skip to content
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(v2): extract site title formatter to theme-common util #3838

Merged
merged 1 commit into from
Nov 30, 2020

Conversation

Simek
Copy link
Contributor

@Simek Simek commented Nov 28, 2020

Motivation

Currently the same function, same formatter is used in various packages to format the siteTitle.

This PR extracts the formatter function to stand alone hook (useTitleFormatter) and moves it to theme-common package.

@slorber My only concern is that after refactor I had to add @docusaurus/theme-common also to docusaurus-theme-search-algolia dependencies. In theory this is the expected that theme-search can be depended on theme-common, but due to the way in theme-common dependencies have been declared, this will mean that Algolia plugin now will fetch all content plugins (https://github.com/facebook/docusaurus/blob/master/packages/docusaurus-theme-common/package.json#L20).

Not sure how big problem it is in the general scope of Docusuasaurs, since this package should not be used without Docusuaurs, but maybe there is a better way to define/setup this. Maybe theme-common should only specify peerDependencies?

Have you read the Contributing Guidelines on pull requests?

Yes.

Test Plan

Changes have been tested using Docusuaurs V2 website locally.

Related PRs

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Nov 28, 2020
@netlify
Copy link

netlify bot commented Nov 28, 2020

Deploy preview for docusaurus-2 ready!

Built without sensitive environment variables with commit d9d651c

https://deploy-preview-3838--docusaurus-2.netlify.app

@slorber
Copy link
Collaborator

slorber commented Nov 30, 2020

Thanks, that does not seem like a big issue to me.

CF the updated theme doc: https://v2.docusaurus.io/docs/api/themes

"main themes" depends on docs,blog, pages, but algolia search is a "theme enhancer" that improves one main theme, so it's not a big deal if it depends on the same things

@slorber slorber merged commit 21572cc into facebook:master Nov 30, 2020
@slorber slorber added the pr: maintenance This PR does not produce any behavior differences to end users when upgrading. label Nov 30, 2020
@slorber slorber changed the title feat(v2): extract site title formatter to theme-common util refactor(v2): extract site title formatter to theme-common util Nov 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA pr: maintenance This PR does not produce any behavior differences to end users when upgrading.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants