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

Amend docs - "add typescript" global warning #6911

Closed
wants to merge 3 commits into from
Closed

Amend docs - "add typescript" global warning #6911

wants to merge 3 commits into from

Conversation

methodbox
Copy link
Contributor

Related to: #6816

Updating blockquote to advise on use of CRA globally vs. npx when using --typescript flag.

@facebook-github-bot
Copy link

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 up 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 the corporate CLA signed.

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

@facebook-github-bot
Copy link

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

@methodbox methodbox changed the title Amend docs add typescript Amend docs - "add typescript" global warning Apr 26, 2019
@amyrlam amyrlam self-assigned this Apr 28, 2019
@amyrlam
Copy link
Contributor

amyrlam commented Apr 28, 2019

@methodbox looks good 👍 checked out the conversation on the linked issue. Can you remove the commits from nagman?

@methodbox
Copy link
Contributor Author

Sure, I’ll take a look at this in the morning.

@@ -16,6 +16,7 @@ npx create-react-app my-app --typescript

yarn create react-app my-app --typescript
```
> Note that having `create-react-app` installed globally can cause conflicts when using `--typescript`. If you've previously installed `create-react-app` globally via `npm install -g create-react-app`, we recommend you uninstall the package using `npm uninstall -g create-react-app` to ensure that `npx` always uses the latest version.
Copy link
Contributor

Choose a reason for hiding this comment

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

This first sentence is not accurate. Having Create React App installed globally doesn't create a "conflict" with the --typescript flag. It just means you're more likely to have an old version installed that doesn't have the --typescript flag. As the last sentence says, you should use npx instead of a global install to ensure you're using an up to date version of Create React App.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've made this change but I'm still not sure how to correctly remove the commits from @nagman.

@methodbox
Copy link
Contributor Author

@methodbox looks good 👍 checked out the conversation on the linked issue. Can you remove the commits from nagman?

I'm a bit confused by this; it looks like this commit has already been merged. Can you clarify? What is the best way to remove these commits? I've found this on Stackoverflow, but not sure: https://stackoverflow.com/questions/16306012/github-pull-request-showing-commits-that-are-already-in-target-branch

@amyrlam
Copy link
Contributor

amyrlam commented Apr 28, 2019

I noticed that you forked from nagman instead of facebook.

image

It may be simplest to redo your fork from here https://github.com/facebook/create-react-app and submit a new PR. (That will also make it easier for you to submit PR's in the future with a synced fork.)

Be sure to do the change that iansu mentioned, which will match what was discussed in #6816 (comment).

@methodbox
Copy link
Contributor Author

I noticed that you forked from nagman instead of facebook.

image

It may be simplest to redo your fork from here https://github.com/facebook/create-react-app and submit a new PR. (That will also make it easier for you to submit PR's in the future with a synced fork.)

Be sure to do the change that iansu mentioned, which will match what was discussed in #6816 (comment).

Okay, oops! I'll resubmit as you said, forking from FB/CRA.

@methodbox
Copy link
Contributor Author

I've created a corrected PR ... forked from the right location this time 🥇

#6945

Closing.

@methodbox methodbox closed this Apr 28, 2019
@lock lock bot locked and limited conversation to collaborators May 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants