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): add @theme-init alias to give access to initial components #2464

Merged
merged 6 commits into from
May 17, 2020

Conversation

lex111
Copy link
Contributor

@lex111 lex111 commented Mar 24, 2020

Motivation

This is a quick working prototype, maybe we need to work on better naming and other things.

I have long wanted to get rid of code duplication in code block components (one basic / one with live block) and, inspired by PR #2455, I realized how this can be achieved by creating a HOC and using the basic code block component as a wrapped component.

To do this, we need a new alias, with which we can get the "initial" component - from the theme itself, not overridden by any plugins.

In this PR, both the code for adding this alias and the refactoring of code block components are simultaneously presented in order to demonstrate it in action. If there is a green light, then I can split this PR into two separate ones.

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

Is an new test needed for this?

See this section on preview website, where basic code blocks and live editor are used.

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.)

@lex111 lex111 added the pr: new feature This PR adds a new API or behavior. label Mar 24, 2020
@lex111 lex111 requested a review from yangshun March 24, 2020 23:31
@lex111 lex111 requested a review from wgao19 as a code owner March 24, 2020 23:31
@docusaurus-bot
Copy link
Contributor

docusaurus-bot commented Mar 24, 2020

Deploy preview for docusaurus-2 ready!

Built with commit bdedcbc

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

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Mar 24, 2020
Copy link
Contributor

@yangshun yangshun left a comment

Choose a reason for hiding this comment

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

Not really understanding this PR well. Why can't you use the @theme-original?

@lex111
Copy link
Contributor Author

lex111 commented Mar 25, 2020

Because using theme-original alias (just like using theme alias) I will not get the theme component (where it was initially defined) from the plugin's code:

'@theme/CodeBlock': 'docusaurus/packages/docusaurus-theme-live-codeblock/src/theme/CodeBlock/index.js',
'@theme-original/CodeBlock': 'docusaurus/packages/docusaurus-theme-live-codeblock/src/theme/CodeBlock/index.js',

While init-theme alias refers to the proper (theme) component (from the theme itself, where it is first defined):

'@theme-init/CodeBlock': 'docusaurus/packages/docusaurus-theme-classic/src/theme/CodeBlock/index.js',

Do you see the difference? Do you understand now? theme-original alias is only useful in userland theme, but in plugin code we need a new alias to reference initial (theme) components.

And so we can use it for HOC.

@lex111
Copy link
Contributor Author

lex111 commented Mar 28, 2020

Any thoughts? Still not clear? It seems to me that this is a good way to get rid of code duplication in the live code block component, because it is annoying to duplicate changes every time.

lex111 referenced this pull request Mar 31, 2020
* feat: support comments for code highlighting

* docs(v2): demonstrate highlighting with comments

* chore: remove debugging console.log

* fix: disable when language is undefined
@lex111
Copy link
Contributor Author

lex111 commented Apr 2, 2020

@yangshun yangshun added this to the v2.0.0-alpha.51 milestone Apr 4, 2020
lex111 added a commit that referenced this pull request Apr 5, 2020
@lex111
Copy link
Contributor Author

lex111 commented Apr 17, 2020

@yangshun I would really not like to close this PR, but I don't see you interested in it :(
So does it make sense for me to resolve further conflicts?

@yangshun yangshun added this to the v2.0.0-alpha.55 milestone May 14, 2020
@lex111
Copy link
Contributor Author

lex111 commented May 17, 2020

@yangshun can you accept this PR? Am I tired of updating it every time according to master branch? As I already mentioned, this is a fairly safe change that will allow us not to duplicate changes in code block components.

@yangshun yangshun merged commit d910ff1 into master May 17, 2020
@yangshun yangshun added pr: maintenance This PR does not produce any behavior differences to end users when upgrading. and removed pr: new feature This PR adds a new API or behavior. labels May 17, 2020
@yangshun yangshun changed the title feat(v2): add @theme-init alias to give access to initial components refactor(v2): add @theme-init alias to give access to initial components May 17, 2020
@lex111 lex111 deleted the lex111/init-theme branch May 17, 2020 15:33
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

4 participants