-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
first pass at integration docs #239
Conversation
✅ Deploy Preview for astro-docs-2 ready! 🔨 Explore the source changes: 93b5d14 🔍 Inspect the deploy log: https://app.netlify.com/sites/astro-docs-2/deploys/6238ca406afbc00008f019aa 😎 Browse the preview: https://deploy-preview-239--astro-docs-2.netlify.app |
de2a03a
to
933e65d
Compare
Whew! Big changes! :) Some specific inlined comments above. More generally and to-do:
|
933e65d
to
3a49cbc
Compare
src/pages/en/migrate.md
Outdated
|
||
Looking ahead to the future, we have already started work on a helpful `astro add NAME` command that will be able to add new integrations to your project with a single command. But until this, this is still is a bit of a manual, one-time process to update your configuration. | ||
|
||
<!-- TODO: Shiki default! --> |
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.
@bholmesdev would love your help here!
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.
I'm not @bholmesdev but I will point out that I expect a significant portion of our existing users who don't even know we ever got the option to switch the default to Shiki... let alone made that switch already.
Whatever goes in the Migration docs should start from the perspective that someone PROBABLY has Prism as a default, and should describe what they can immediately do to revert if a) they liked their Prism, or b) if they just don't want to deal with it YET (ie just switch to prism
in config if the new default is causing any problems or throwing you off). They should already have a Prism theme installed or linked to, because they would have needed it, so that should "just work" for them.
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.
Yup, as of right now it should just be "hey, if you want to keep using prism that's fine, just add/change this one line of config." Will leave to Ben to write it up
Looks great! We need to rename the first "Integrations" tab (for showcases etc) under the Start Here section in favour of the new one, might get confusing otherwise. |
Took another pass! Feeling a lot better about this one, thank everyone for the feedback so far. @sarah11918 ready for another pass! |
- Unlock React, Vue, Svelte, Solid, and support for other popular UI frameworks. | ||
- Integrate popular tools like Tailwind, Turbolinks, and Partytown with only a few lines of code. | ||
- Add new features to your build, like automatic sitemap generation. | ||
- Write custom hooks into the build process, dev server, and more. |
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.
This line is a bit odd to read - maybe re-write it as something like "write custom code that hooks into the build process, dev server, and more."
|
||
**Astro Integrations** add new functionality and behaviors for your project with only a few lines of code. You can write a custom integration yourself, or grab popular ones from npm. | ||
|
||
- Unlock React, Vue, Svelte, Solid, and other popular UI frameworks. |
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.
I wonder about a note/warning pointing out, and making it explicit that you are not installing REACT, but rather Astro's React Integration?
(Since we do mention installing "any npm package" I can see someone not reading the tutorial and just jumping to install React directly.) Or, something like:
- Unlock React, Vue, Svelte, Solid and more by installing Astro integrations for popular UI frameworks.
If you miss this step, don't worry, Astro will warn you during startup if any missing peer dependencies are required but not found in your project. | ||
|
||
Managing your own peer dependencies may require more work, but it also lets you control what versions of packages you use for things like React, Tailwind, and more. | ||
|
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.
I would personally reorder the flow of this section, empahsizing the benefit of managing peer dependencies first, then you'll get a warning, the what to do, then an example at the end. Just a personal preference, but something like:
Managing your own peer dependencies may require more work, but it also lets you control what versions of packages you use for things like React, Tailwind, and more.
Astro will warn you during start up if any missing peer dependencies are required, but not found in your project. If you see any "missing peer dependencies" messages during the install step, you may also need to install those packages in your project. (Not all package managers will install them for you automatically.)
React, for example, is a peer dependency of the @astrojs/react
integration. npm (v7+) will install the required React packages for you automatically when you install @astrojs/react
. In other package managers (including older versions of npm) you will need to install these yourself, seperately:
# Install any missing peer dependencies for an integration
npm install --save-dev react react-dom
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.
Ah, now that the peer dependency this has been determined to be more of a "react to this issue" than "hey, just so you know"... I get why maybe addressing the error message right away makes more sense. :) I'm OK with the original flow, since it's more reactive to the error message!
Some small, specific comments above, but otherwise LGTM! I will use this to update /en/core-concepts/framework-components# and /en/guides/manual-setup/#5-create-astroconfigmjs |
Merged this into a new |
The integration docs are ready for review! This is definitely still a first draft, so all feedback welcome, especially around what information should live in the "guide" vs. the "reference".
There are some TODOs left that I think could even be tackled post-merge, but a few of them are probably still a priority for this PR.