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

feat(v2): implement theme component swizzling #1435

Merged
merged 4 commits into from
May 6, 2019
Merged

feat(v2): implement theme component swizzling #1435

merged 4 commits into from
May 6, 2019

Conversation

yangshun
Copy link
Contributor

@yangshun yangshun commented May 6, 2019

Motivation

I realized theming can be done almost solely via webpack aliases. Drawing lots of inspiration from VuePress, this is how we will do theming:

  • A theme is a plugin and its main purpose is to provide components and set webpack aliases for them. It can implement the content fetching lifecycles and we don't prevent if from doing that, but it's probably a bad idea and people shouldn't do it. Why I differentiate them from plugins is because I want all the themes to only be initialized and its configureWebpack method to run after all the plugins, that way, the component overriding order is guaranteed.
  • The content plugins will provide its corresponding components along with the respective aliases.
  • For components that aren't tied to any content, put them under a default theme.
  • If users want to override any component, they can add a component under the same name in website/theme/ dir, e.g. website/theme/DocPaginator.js.
  • Presets can now include themes on top of plugins (and only limited to themes and plugins).

RFC

This approach works well, but is not without drawbacks. @endiliey I wanna get your comments on the following:

  • If someone overrides a theme component and later removes the theme, that overridden component will still be there in the code but doesn't do anything. It wouldn't get bundled into the final bundle but dead code can be a maintenance burden in future.
  • We now have 3 things to teach users - plugins, themes, and presets. Themes are plugins, so that's easy to teach. Presets is a collection of themes and plugins but its shape is similar to docusaurus.config.js and people might get confused and start putting other keys into presets. I wonder if it'll be easier to just make everything work the same way - everything is a plugin and plugins can include more plugins and themes. IMO this will just create a huge mess of plugin inheritance and I rather try to keep the hierarchy flat. WDYT?
  • I currently provide the theme components for docs and blog within its content plugin because they're related and a content plugin cannot work without the default components. Do you think it's cleaner to just make those components as part of a separate theme e.g. docusaurus-theme-blog and docusaurus-theme-docs and have the preset include the individual themes? The original rationale for putting them together was that if someone just wants the blog, they can include one plugin and they're good to go, rather than installing a plugin AND a theme.
  • Almost every user that needs just a bit of customization will need to override Layout.js so I wonder if it's better putting it as part of the default theme vs scaffolding for people.
  • People will have to use Infima CSS as the default theme. If they want to user another CSS framework, they'll have to override almost all the components 😱
  • Because the webpack aliases are generated only once when the dev server starts, any addition or removal will require restarting the dev server. VuePress also suffers from this problem. There's probably a way around this though, by watching the files and restarting the dev server when there are changes

Follow Up

  • Remove hardcoding from Footer.js
  • Separate search into its own plugin
  • Implement more plugin lifecycles that allow wrapping the entire page
  • Refactor siteConfig and allow options to be passed into themes
  • Extract Infima stylesheet and include it into default theme instead of Docusaurus core

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

Tried component overriding locally. Works well! 😄Try it for yourself!

Related PRs

(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/facebook/docusaurus, and link to your PR here.)

@yangshun yangshun requested a review from endiliey as a code owner May 6, 2019 06:41
@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label May 6, 2019
@docusaurus-bot
Copy link
Contributor

docusaurus-bot commented May 6, 2019

Deploy preview for docusaurus-2 ready!

Built with commit 8605b05

https://deploy-preview-1435--docusaurus-2.netlify.com

@docusaurus-bot
Copy link
Contributor

docusaurus-bot commented May 6, 2019

Deploy preview for docusaurus-preview ready!

Built with commit 8605b05

https://deploy-preview-1435--docusaurus-preview.netlify.com

Copy link
Contributor

@endiliey endiliey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current approach looks OK.

I've tested locally and made several changes

Add theme component overriding support for more than one level deep of directory.

Previously it can only support 1 level deep of directory :O

I renamed theme-default to theme-classic and preset-default as preset-classic.

My reasoning is because it's actually not really a default and it has to be installed. We are also going towards a general purpose static site gen now. I don't think everyone want a navbar and a footer (my blog doesn't want it).

I've patched the PR to add a real default/fallback theme components such as Loading, NotFound and Layout that has no css dependencies with Infima and is very minimal. It's located here
https://github.com/facebook/Docusaurus/tree/theme/packages/docusaurus/lib/client/theme-fallback

We need it because stuff like react-loadable need a Loading component or it will throw an error. It can still be overriden in theme plugins, or theme folder in site directory.

Here is an example:
Docs plugin + theme-classic
image

Only docs plugin (without the theme-classic being installed). No error
Notice that there is no NavBar or Footer!
no navbar docs

So my blog site (endiliey.com) doesn't need to install theme-classic. I just need to install blog-plugin and I'm good to go.

Because the webpack aliases are generated only once when the dev server starts, any addition or removal will require restarting the dev server. VuePress also suffers from this problem. There's probably a way around this though, by watching the files and restarting the dev server when there are changes

Actually i noticed this as well when trying it out too. Anyway, Gatsby is facing this as well on their component shadowing. See their issue. There are better workarounds such as building a fake component that just basically do an export default from /real/path/. (no dev server restart, + reloading all works)
But, I think it's ok to leave it as it is now.


const requiredComponents = ['Loading', 'NotFound'];
Copy link
Contributor

@endiliey endiliey May 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a reason why this was added. We had to make sure these components (e.g: Loading, NotFound) always exist whether the @docusaurus/theme-default is installed or not. This will break my site (e.g: endiliey.com)

I will patch this PR to move several essential components to core

Edit: patched

const fileStats = await fs.stat(filePath);
const fileExt = fileStats.isFile() ? path.extname(filePath) : '';
const fileName = path.basename(filePath, fileExt);
alias[`@theme/${fileName}`] = filePath;
Copy link
Contributor

@endiliey endiliey May 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this only works for 1 level directory deep. I'll patch this

Edit: patched

@endiliey endiliey merged commit eedd4c4 into master May 6, 2019
@endiliey
Copy link
Contributor

endiliey commented May 6, 2019

I merged this out cos we can do the next one on next PR for easier review. We might want to allow eject from theme plugins so that user don't have to copy paste the file code & structure when trying to component override

@endiliey
Copy link
Contributor

endiliey commented May 6, 2019

Another idea that I have. Instead of having the theme plugin to resolve alias everytime like this

https://github.com/facebook/Docusaurus/blob/3298d8cd231835a74abe85649bc20c3296fe3711/packages/docusaurus-theme-classic/src/index.js#L22-L35

We can provide a plugin lifecycle API like getPathToWatch

get themePath() {
  return '/path/to/the/theme/folder'
}

Then we'll recursively go to that folder and alias it. We already do it in loadTheme anyway..

This can make it easy to do yarn eject [plugin] so that user can eject the theme component of that plugin to be copied to their theme folder in siteDir.

I'll try to work on a POC

Edit: see #1436

@endiliey
Copy link
Contributor

endiliey commented May 6, 2019

I wonder if it'll be easier to just make everything work the same way - everything is a plugin and plugins can include more plugins and themes. IMO this will just create a huge mess of plugin inheritance and I rather try to keep the hierarchy flat. WDYT?

Maybe its better to just keep the hierarchy flat. But do presets really need themes and plugins ? Can it be just a flat array of plugins

@yangshun yangshun deleted the theme branch May 6, 2019 15:23
@yangshun
Copy link
Contributor Author

yangshun commented May 6, 2019

Add theme component overriding support for more than one level deep of directory

How would you differentiate between

Foo.js
Bar/index.js? // <- Will this create an alias as @theme/Bar/index? That's inconsistent.

I renamed theme-default to theme-classic and preset-default as preset-classic

Fair enough!

Only docs plugin (without the theme-classic being installed). No error

Maybe I should call it theme-base or something instead, but they could provide their own @theme/Layout and customize whether they want the Navbar or Footer. I'm not too sure whether this is the best idea yet so I'm open to your suggestion too.

We can provide a plugin lifecycle API like getPathToWatch

I thought about this too but I think I prefer it to be explicit because the theme might not want to expose every component.

@yangshun yangshun changed the title feat(v2): implement theme component overriding feat(v2): implement theme component swizzling May 7, 2019
shakcho pushed a commit to shakcho/Docusaurus that referenced this pull request May 17, 2019
* feat(v2): implement component overriding

* siteDir theme overriding should work for > 1 level directory

* fallback for essential component like Loading

* rename default -> classic
yangshun pushed a commit that referenced this pull request Jul 20, 2019
* docs: make navbar less cluttered

* misc(v2): rename components (#1434)

* misc(v2): clean up work

* misc(v2): rename components

* misc(v2): rename Blog components

* refactor(v2): remove page plugin component

* chore(v2): optimize webpack config

* feat(v2): implement theme component overriding (#1435)

* feat(v2): implement component overriding

* siteDir theme overriding should work for > 1 level directory

* fallback for essential component like Loading

* rename default -> classic

* fix(v2): add missing layout alias on theme-classic plugin

* feat(v2): easier plugin theme components swizzling (#1436)

* feat(v2): easier plugin theme components override

* add context

* refactor again

* rename eject -> swizzle

* nits

* fix(v2-cli): passes the missing host option to start (#1439)

* feat(v2): Algolia search theme (#1440)

* feat(v2): Algolia search plugin

* patch PR #1440 (#1441)

* alternative implementation

* typo

* refactor noop

* rename SearchAlgolia -> SearchBar

* changes.md

* refactor(v2): move headerLinks -> themeConfig & rm dead code (#1442)

* refactor(v2): move headerLinks -> themeConfig & rm dead code

* rm -rf dead code

* chore(v2): better chunk naming

* refactor(v2): add flowtype + refactor test (#1443)

* chore(v2): add flow setup

* nits

* fix

* add flow-typed

* ignore compiled library

* fix error

* fix typing

* fix module name mapper

* setup for @docusaurus/core

* dont try remove type without @flow

* fix can't find @docusaurus/utils

* fix test

* remove obscure relative paths

* more refactoring

* add typing for server/load/theme.js

* no need to ship .flow source

* chore(v2): rm flowtype (#1444)

* chore(v2): tweak eslint config (#1445)

* chore: point website-1.x to correct version so that it can pick latest code

* feat(v2): meta description (#1447)

* feat(v2): meta description

* add description for blog as well

* fix non-descriptive text link

* remove font awesome

* switch front-matter -> gray-matter

* fix(v2): docsearch a11y issue (#1449)

* refactor(v2): blog data revamp (#1450)

* refactor(v2): blog data revamp

* fix(v2): fix incorrect blog total count

* misc: remove console.log

* feat(v2): export frontMatter as an object within MDX file (#1451)

* refactor. Don't confuse metadata & frontmatter

* export frontMatter in content itself

* nits

* nits name

* dont truncate first four lines in blog

* fix(v2): transpiling, window scroll and console error (#1452)

* chore(v2): better error message style (#1454)

* chore(v2): remove docsearch a11y workaround

* fix(v2): slugify tags

* feat(v2): blog tags (#1453)

* feat(v2): blog tags

* feat(v2): blog tags

* chore(v2): use remark-slug so that rightToc can benefit from it

* fix: right TOC should not strip special chars (#1458)

* fix: right TOC should not strip special chars

* nits

*  fix(v2): handle non existent blog, docs, pages (#1459)

* fix(v2): handle non existent blog, docs, pages

* nits

* feat(v2): list blog tags on posts (#1456)

* feat(v2): list blog tags on posts

* fix date handling on blog header

* fix console log error due to non unique key

* test(v2): test different type of sidebar item

* chore(v2): fix typo

* v2.0.0-alpha.14

* fix(v2): fix wrong dependency problem (#1460)

* v2.0.0-alpha.15

* Chore(v2): use alias instead of relative path for blogpost

* feat(v2): theme config for Footer (#1461)

* feat(v2): theme config for Footer

* fix: dont show footer if themeConfig.footer is undefined
* Import fresh docusaurus.config.js for better hot reload

* chore(v2): update dependencies (#1462)

* chore(v2): update dependencies

* nits

* v2.0.0-alpha.16

* fix(v2): fix cannot import css from node_modules in userland (#1463)

* docs: showcase user Express Validator (#1464)

* docs: sort user with alphabetical (#1465)

* chore: fix typo (#1466)

* docs: showcase user tipsi-stripe (#1423)

* docs: make it clear in the tutorial where the `docs` folder is (#1468)

* Make it clear where the `docs` folder is

It was not clear, to the beginner user—who this tutorial is for—where the `docs` folder was . The only reason I know this is because I'm a beginner user and I tried for too many minutes to find the `docs` folder inside the `website` folder. I had this assumption because the previous example is offered under the assumption that you're in the `website` folder.

Feel free to change the wording, I just want to make it clear where you should be looking, if you're new.

* Update tutorial-create-pages.md

* docs: add hint for linking dependencies time (#1470)

In this tutorial we assume that the user may or may not have used Node.js. It would follow then, based on how long it takes the "Linking dependencies" step to complete—243.61s for me—that we give them a hint that it might take a minute.

* doc: mention HTTPS approach in tutorial git clone step (#1471)

* doc: change tutorial git to suggest HTTPS

Since we're instructing the user to create a new repository, it might be a safe bet to assume that they don't have their SSH keys set up. HTTPS might be a better option in this context.

* docs: keep ssh add https

@hongarc said, "How about keep `SSH` and add `HTTPS`." This is one way to do that.

* Update tutorial-setup.md

* docs: normalized spelling of `web server` (#1473)

* docs: clarify location of sidebars.json (#1472)

In #1468 we clarify where the `docs` folder is. 
Here, we make it clear where `sidebars.json` is.

* fix: missing cli commands (#1478)

* fix: missing cli commands

* centralize

* feat(v2): move navbar config into themeConfig (#1477)

* feat(v2): move navbar config into themeConfig

* misc: fix tests

* fix: support external url for logo

* docs(v2): CLI docs (#1476)

* WiP: CLI docs

* Tweak word choices for CLI docs

- Use the word swizzle directly
- Follow variable convention for shell

* Resolve docs discussion

* Update cli.md

* fix(v2): should be able to build even if static folder doesnt exist (#1479)

* chore: remove noWatch cli options because you cant disable watch in wds (#1480)

* docs: update StreamPipes logo (#1481)

* docs: showcase user Ax (#1483)

* docs: remove pinned for Taro (#1482)

* docs: fix typo for `docs` folder (#1484)

* docs: fix typo for `docs` folder

* docs: request change for #1484

* Update api-doc-markdown.md

* docs: add some showcase user of `facebook` (#1486)

* docs: showcase user Idb

* docs: showcase user Netconsd

* docs: showcase user Redex

* added autoprefixer code with postcss loader

* removed unused changes

* added correct importloader and removed unused packages and conf
yangshun pushed a commit that referenced this pull request Aug 4, 2019
* docs: make navbar less cluttered

* misc(v2): rename components (#1434)

* misc(v2): clean up work

* misc(v2): rename components

* misc(v2): rename Blog components

* refactor(v2): remove page plugin component

* chore(v2): optimize webpack config

* feat(v2): implement theme component overriding (#1435)

* feat(v2): implement component overriding

* siteDir theme overriding should work for > 1 level directory

* fallback for essential component like Loading

* rename default -> classic

* fix(v2): add missing layout alias on theme-classic plugin

* feat(v2): easier plugin theme components swizzling (#1436)

* feat(v2): easier plugin theme components override

* add context

* refactor again

* rename eject -> swizzle

* nits

* fix(v2-cli): passes the missing host option to start (#1439)

* feat(v2): Algolia search theme (#1440)

* feat(v2): Algolia search plugin

* patch PR #1440 (#1441)

* alternative implementation

* typo

* refactor noop

* rename SearchAlgolia -> SearchBar

* changes.md

* refactor(v2): move headerLinks -> themeConfig & rm dead code (#1442)

* refactor(v2): move headerLinks -> themeConfig & rm dead code

* rm -rf dead code

* chore(v2): better chunk naming

* refactor(v2): add flowtype + refactor test (#1443)

* chore(v2): add flow setup

* nits

* fix

* add flow-typed

* ignore compiled library

* fix error

* fix typing

* fix module name mapper

* setup for @docusaurus/core

* dont try remove type without @flow

* fix can't find @docusaurus/utils

* fix test

* remove obscure relative paths

* more refactoring

* add typing for server/load/theme.js

* no need to ship .flow source

* chore(v2): rm flowtype (#1444)

* chore(v2): tweak eslint config (#1445)

* chore: point website-1.x to correct version so that it can pick latest code

* feat(v2): meta description (#1447)

* feat(v2): meta description

* add description for blog as well

* fix non-descriptive text link

* remove font awesome

* switch front-matter -> gray-matter

* fix(v2): docsearch a11y issue (#1449)

* refactor(v2): blog data revamp (#1450)

* refactor(v2): blog data revamp

* fix(v2): fix incorrect blog total count

* misc: remove console.log

* feat(v2): export frontMatter as an object within MDX file (#1451)

* refactor. Don't confuse metadata & frontmatter

* export frontMatter in content itself

* nits

* nits name

* dont truncate first four lines in blog

* fix(v2): transpiling, window scroll and console error (#1452)

* chore(v2): better error message style (#1454)

* chore(v2): remove docsearch a11y workaround

* fix(v2): slugify tags

* feat(v2): blog tags (#1453)

* feat(v2): blog tags

* feat(v2): blog tags

* chore(v2): use remark-slug so that rightToc can benefit from it

* fix: right TOC should not strip special chars (#1458)

* fix: right TOC should not strip special chars

* nits

*  fix(v2): handle non existent blog, docs, pages (#1459)

* fix(v2): handle non existent blog, docs, pages

* nits

* feat(v2): list blog tags on posts (#1456)

* feat(v2): list blog tags on posts

* fix date handling on blog header

* fix console log error due to non unique key

* test(v2): test different type of sidebar item

* chore(v2): fix typo

* v2.0.0-alpha.14

* fix(v2): fix wrong dependency problem (#1460)

* v2.0.0-alpha.15

* Chore(v2): use alias instead of relative path for blogpost

* feat(v2): theme config for Footer (#1461)

* feat(v2): theme config for Footer

* fix: dont show footer if themeConfig.footer is undefined
* Import fresh docusaurus.config.js for better hot reload

* chore(v2): update dependencies (#1462)

* chore(v2): update dependencies

* nits

* v2.0.0-alpha.16

* fix(v2): fix cannot import css from node_modules in userland (#1463)

* docs: showcase user Express Validator (#1464)

* docs: sort user with alphabetical (#1465)

* chore: fix typo (#1466)

* docs: showcase user tipsi-stripe (#1423)

* docs: make it clear in the tutorial where the `docs` folder is (#1468)

* Make it clear where the `docs` folder is

It was not clear, to the beginner user—who this tutorial is for—where the `docs` folder was . The only reason I know this is because I'm a beginner user and I tried for too many minutes to find the `docs` folder inside the `website` folder. I had this assumption because the previous example is offered under the assumption that you're in the `website` folder.

Feel free to change the wording, I just want to make it clear where you should be looking, if you're new.

* Update tutorial-create-pages.md

* docs: add hint for linking dependencies time (#1470)

In this tutorial we assume that the user may or may not have used Node.js. It would follow then, based on how long it takes the "Linking dependencies" step to complete—243.61s for me—that we give them a hint that it might take a minute.

* doc: mention HTTPS approach in tutorial git clone step (#1471)

* doc: change tutorial git to suggest HTTPS

Since we're instructing the user to create a new repository, it might be a safe bet to assume that they don't have their SSH keys set up. HTTPS might be a better option in this context.

* docs: keep ssh add https

@hongarc said, "How about keep `SSH` and add `HTTPS`." This is one way to do that.

* Update tutorial-setup.md

* docs: normalized spelling of `web server` (#1473)

* docs: clarify location of sidebars.json (#1472)

In #1468 we clarify where the `docs` folder is. 
Here, we make it clear where `sidebars.json` is.

* fix: missing cli commands (#1478)

* fix: missing cli commands

* centralize

* feat(v2): move navbar config into themeConfig (#1477)

* feat(v2): move navbar config into themeConfig

* misc: fix tests

* fix: support external url for logo

* docs(v2): CLI docs (#1476)

* WiP: CLI docs

* Tweak word choices for CLI docs

- Use the word swizzle directly
- Follow variable convention for shell

* Resolve docs discussion

* Update cli.md

* fix(v2): should be able to build even if static folder doesnt exist (#1479)

* chore: remove noWatch cli options because you cant disable watch in wds (#1480)

* docs: update StreamPipes logo (#1481)

* docs: showcase user Ax (#1483)

* docs: remove pinned for Taro (#1482)

* docs: fix typo for `docs` folder (#1484)

* docs: fix typo for `docs` folder

* docs: request change for #1484

* Update api-doc-markdown.md

* docs: add some showcase user of `facebook` (#1486)

* docs: showcase user Idb

* docs: showcase user Netconsd

* docs: showcase user Redex

* added css for feedback page contrast issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants