-
Notifications
You must be signed in to change notification settings - Fork 420
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(react-integration): revamp into complete guide #964
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
||
**Type: `string`** | ||
|
||
The title of the Stencil package where components are available for consumers (i.e. the `name` property value in your Stencil project's `package.json`). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Optional word smithing to use some of the same nomenclature as the rest of the doc:
The title of the Stencil package where components are available for consumers (i.e. the `name` property value in your Stencil project's `package.json`). | |
The name of the Stencil package where components are available for consumers (i.e. the value of the `name` property in your Stencil component library's `package.json`). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rwaskiewicz I like this change, but this same thing probably needs to happen in the Angular and Vue docs as well. So, I'll hold off on making this swap until we merge all these back into one PR so then I can just change them all at once
npm link | ||
From the `packages/` directory, run the following command to create a starter React app: | ||
|
||
```bash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we convert this to an npm2yarn block?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First off, I decided to swap this off of create-react-app
to a Vite React app instead in 4d88bc3. Did this for two reasons:
- The command for generating w/
create-react-app
has some issues in Yarn environments create-react-app
is kinda dead, so it's easy enough for us to move away from it
Even so, we can't use npm2yarn
here because that plugin can't correctly convert certain syntaxes (like npm create
/npm init
) to yarn syntax. It tries its best but then spits out a comment in the generated yarn block along the lines of "couldn't auto-convert command". I believe it's related to this issue in the docusaurus repo. I marked a TODO comment in the docs tho for us to look into using npm2yarn if/when that is ever resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! 🚢
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me overall, just noticed a few quick text things
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
Updates the docs for the React framework wrapper to follow the same structure as the Angular docs