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

docs(v2): wrap all plugin imports in require.resolve() #2941

Merged
merged 3 commits into from
Jun 16, 2020

Conversation

TomBrien
Copy link
Contributor

Motivation

(Write your motivation here.)

As per the breaking change in 2.0.0-alpha56 plugin call need to be wrapped in require.resolve(...). Add this to the examples.

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Bonus points for screenshots and videos!)

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

As per breaking change in 2.0.0-alpha0.56
@TomBrien TomBrien requested a review from yangshun as a code owner June 15, 2020 20:53
@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Jun 15, 2020
@docusaurus-bot
Copy link
Contributor

docusaurus-bot commented Jun 15, 2020

Deploy preview for docusaurus-2 ready!

Built with commit 9f79ac6

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

@slorber
Copy link
Collaborator

slorber commented Jun 16, 2020

Thanks @TomBrien

Do you mind updating as well the /docs folder? otherwise on next release your docs changes will be reverted. The versioned_docs folder is for already released versions, and will not be used for next releases.

@slorber slorber self-requested a review June 16, 2020 09:18
@TomBrien
Copy link
Contributor Author

My bad just hit the edit on GitHub button. Sure will move that over now

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.

I think this only serves as a workaround due to a bug in the current version, and will not be necessary after we release the next alpha version is that right?. cc @SamChou19815

@yangshun yangshun changed the title Wrap all plugin imports in require.resolve() docs(v2): wrap all plugin imports in require.resolve() Jun 16, 2020
@yangshun yangshun added the pr: documentation This PR works on the website or other text documents in the repo. label Jun 16, 2020
@SamChou19815
Copy link
Contributor

I think this only serves as a workaround due to a bug in the current version, and will not be necessary after we release the next alpha version is that right?. cc @SamChou19815

Yes, but I think it makes sense to make the change only in alpha-56 versioned docs.

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.

Makes sense, since this bug is only present in alpha.56. Could you make the necessary changes?

Ironically the first commit is what we want -e187af0

@TomBrien
Copy link
Contributor Author

Will revert the second commit :)

@yangshun yangshun merged commit e97d2ea into facebook:master Jun 16, 2020
@yangshun
Copy link
Contributor

Thank you very much @TomBrien!

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: documentation This PR works on the website or other text documents in the repo.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants