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): display yarn and npm command on website #2037

Merged
merged 3 commits into from
Nov 24, 2019
Merged

Conversation

endiliey
Copy link
Contributor

@endiliey endiliey commented Nov 23, 2019

Motivation

Show npm and yarn command on website so our docs is more friendly to both package manager users.

I created a naive plugin to convert npm commands to yarn. Will never make it official because my solution to convert npm to yarn is naive. Making it official also means = more bug reports, i have to write tests, etc.

This is also not the best remark plugin in the world. Still fit our use case

Have you read the Contributing Guidelines on pull requests?

yes

Test Plan

image

npm2yarn

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Nov 23, 2019
@@ -420,9 +420,6 @@ The following example is how you can have multi-language code tabs in your docs.

And you will get the following:

import Tabs from '@theme/Tabs';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No need to redeclare because markdown above alrd has npm2yarn meta which will import the Tabs component for us

@endiliey endiliey added pr: documentation This PR works on the website or other text documents in the repo. pr: maintenance This PR does not produce any behavior differences to end users when upgrading. labels Nov 23, 2019
@docusaurus-bot
Copy link
Contributor

docusaurus-bot commented Nov 23, 2019

Deploy preview for docusaurus-2 ready!

Built with commit ff834a5

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

@docusaurus-bot
Copy link
Contributor

docusaurus-bot commented Nov 23, 2019

Deploy preview for docusaurus-preview ready!

Built with commit ff834a5

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

@endiliey endiliey added the status: don't merge yet This PR is completed, but we are still waiting for other things to be ready. label Nov 23, 2019
@endiliey endiliey removed the status: don't merge yet This PR is completed, but we are still waiting for other things to be ready. label Nov 23, 2019
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.

This is such a cool idea. Were you inspired by react-tracked? 😏

Docusaurus Themes' components are designed to be replaceable. To make it easier for you, we created a command for you to replace theme components called `swizzle`.

To swizzle a component for a theme, run the following command in your doc site:

```shell
$ docusaurus swizzle [theme name] [component name]
docusaurus swizzle <theme name> [component name]
Copy link
Contributor

Choose a reason for hiding this comment

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

Side question - Does this actually work if they didn't install docusaurus globally? Or do they have to do npm run docusaurus swizzle ...?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah you are right. I only followed prevs docs

node.children.forEach(transformer);
}
if (node.type === 'root' && transformed) {
node.children.unshift({
Copy link
Contributor

Choose a reason for hiding this comment

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

If the MDX file already import @theme/Tabs this is gonna add an extra import statement right? But I guess it's fine, don't think repeated statements matter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah. the logic could have been better but this is easily avoidable with human error prevention.

Idw to make it too complex such that i have to check whole markdown if the import exist with babel parse. Its just increasing parsing time.

Hence why this will never be official

npmCode
.replace(/^npm i$/gm, 'yarn')
// install: 'npm install --save foo' -> 'yarn add foo'
.replace(/npm install --save/gm, 'yarn add')
Copy link
Contributor

Choose a reason for hiding this comment

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

Wanna handle the npm install --save-dev scenario too? Although we won't likely need it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

most likely we wont have dev i guess. Its hard to find perfect solution to convert npm 2 yarn

// global install: 'npm i' -> 'yarn'
return (
npmCode
.replace(/^npm i$/gm, 'yarn')
Copy link
Contributor

Choose a reason for hiding this comment

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

Why only this line has the ^?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

npm i => yarn
npm install —save => yarn add

Because if dont have ^ then it can be
yarnnstall —save 😂😂

If we replace the order then its fine but i did it so its explicit if its a “project reinstall”.

Thats why this is such a naive npm2yarn converter. Ive checked there is no such npm pkg to convert

Copy link
Contributor Author

Choose a reason for hiding this comment

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

npm i => yarn
npm install —save => yarn add

Because if dont have ^ then it can be
yarnnstall —save 😂😂

If we replace the order then its fine but i did it so its explicit if its a “project reinstall”.

Thats why this is such a naive npm2yarn converter. Ive checked there is no such npm pkg to convert

// install: 'npm install --save foo' -> 'yarn add foo'
.replace(/npm install --save/gm, 'yarn add')
// run command: 'npm run start' -> 'yarn run start'
.replace(/npm run/gm, 'yarn run')
Copy link
Contributor

Choose a reason for hiding this comment

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

If I'm not wrong yarn will try to run defined npm scripts if it's not a recognized yarn command. Maybe you can just replace with yarn instead of yarn run.

@yangshun yangshun merged commit c533adc into master Nov 24, 2019
@yangshun yangshun deleted the endi/npm2yarn branch November 24, 2019 02:32
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. 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.

4 participants