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

polish(v2): use npm-to-yarn for the npm2yarn remark plugin #2428

Merged
merged 5 commits into from
Mar 24, 2020
Merged

polish(v2): use npm-to-yarn for the npm2yarn remark plugin #2428

merged 5 commits into from
Mar 24, 2020

Conversation

nebrelbug
Copy link
Contributor

@nebrelbug nebrelbug commented Mar 19, 2020

Motivation

Docusaurus' previous NPM-Yarn code tab generation was very basic and missed many conversions. I extracted some of the logic into a new npm package (npm-to-yarn) and made the code more robust.

Have you read the Contributing Guidelines on pull requests?

Yep

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

When serving on localhost, NPM to Yarn code tab generation works in almost all cases (though for some reason, it seems to fail, as did the last implementation, on one-line codeblocks? I think it's a problem with the Remark plugin)

Actually, the problems I observed previously aren't there any more. Conversions seem to be working perfectly

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

None yet

@facebook-github-bot
Copy link
Contributor

Hi @nebrelbug!

Thank you for your pull request and welcome to our community.We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file.

In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@docusaurus-bot
Copy link
Contributor

docusaurus-bot commented Mar 19, 2020

Deploy preview for docusaurus-2 ready!

Built with commit 7a44aae

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

@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

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

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@nebrelbug
Copy link
Contributor Author

Strangely, the bottom two npm2yarn codeblocks on the Installation page do not convert when running a local server on my device, but do convert correctly on the deploy preview (which I can confirm is using my changes).

This is with the most recent changes, and testing both /docs/ and /docs/next

@nebrelbug
Copy link
Contributor Author

After fixing a few bugs in my NPM package, I can confirm that I didn't see any failed conversions while browsing the docs (except the codeblock that says npm docusaurus --version, which isn't actually correct)

@nebrelbug nebrelbug marked this pull request as ready for review March 23, 2020 22:43
@yangshun yangshun changed the title Use npm-to-yarn for the npm2yarn remark plugin polish(v2): use npm-to-yarn for the npm2yarn remark plugin Mar 24, 2020
@yangshun yangshun added the pr: polish This PR adds a very minor behavior improvement that users will enjoy. 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.

Woohoo thank you!

@yangshun yangshun merged commit a8a4fe4 into facebook:master Mar 24, 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: polish This PR adds a very minor behavior improvement that users will enjoy.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants