-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
fix(integrations-guide): add/format API references blocks #12319
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
fix(integrations-guide): add/format API references blocks #12319
Conversation
✅ Deploy Preview for astro-docs-2 ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
Lunaria Status Overview🌕 This pull request will trigger status changes. Learn moreBy default, every PR changing files present in the Lunaria configuration's You can change this by adding one of the keywords present in the Tracked Files
Warnings reference
|
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 looks great, @ArmandPhilippot ! I've reviewed for syntax, and left thoughts about Partytown.
It would be great if someone had thoughts on the db
and mdx
(recmaPlugins
) types, since that's out of my comfort zone!
Co-authored-by: Sarah Rainsberger <[email protected]>
Thanks Sarah! I've answered for partytown. Now, for Maybe @Adammatthiesen (if you have time to look at it) would have an idea for the For Adam, to avoid you reading all the details: In the description, we had But for |
|
||
<p> | ||
|
||
**Type:** `PluggableList`<br /> |
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.
But for mdx I don't know who we could ask... I hesitated to use unified.PluggableList, but I wasn't sure this would be more explicit for people.
Let's ask @delucis !
(Armand's concern is regarding the entry for recmaPlugins
: "PluggableList is not an Astro type but a unified one and I don't think we reexport it, but I don't have a better idea to describe the type.)
Yeah, im not sure where the original docs writer got that type from 😅 But the new description looks correct, since there really is no proper external type for that. (as currently ALL of the |
Thanks @Adammatthiesen
I think the idea was to illustrate a single column configured inside
Well, I was able to import
even if |
yeah... there is a few very specific types available, but the primary table/column types are not at the moment. For now probably fine to just keep the Mock Column type as long as we are explaining that the user is using a existing table's column (as shown in the code snippet example) |
Co-authored-by: Sarah Rainsberger <[email protected]>
Co-authored-by: Sarah Rainsberger <[email protected]>
@ArmandPhilippot Shall we NWTWWHB this one and get it merged? I know there was one outstanding question, but maybe we merge as-is then change if we find it's not helpful or have a better idea? |
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'll give an explicit approval that I'm happy with what I reviewed!
@sarah11918 If someone is using a So yes, I think we can merge this one! 🫡 |
Description (required)
integrations-guide/db
to remove the inline types and to make the description more accurate based on the types.This looks like a big update but it's not (each block is ~4-7 added lines) that's why I've updated all the integrations guides in one PR.
What you might want to double check is
foreignKeys
indb
,recmaPlugins
inmdx
and the options inpartytown
(see the details for why). I think the choices are reasonable but you might have another opinion!Show details
For each integration API, I added a link so you can easily check there is no errors. I've added some comments for some to explain my choice.
In
alpinejs
:entrypoint
, see [ci] release astro#9802In
db
:columns
: thankfully the type is exported from@astrojs/db/types
so we don't need something more complexindexes
: we could usekeyof ColumnsConfig
foron
, but I think keepingstring
as we had before in the description is less confusing, so I reused that.foreignKeys
: this one is complex, we don't have a type to reference a single column defined incolumns
. In the description, we hadColumn
but the type doesn't exist. I don't have a better idea so I reused that, NWTWWHB. And I updated the description to be more accurate (the type allows a single column, this is not always an array).isDbError()
, see db: exposeisDbError()
utility astro#10498In
markdoc
: (technically speaking, both are not set tofalse
by default but rather left undefined... which would have the same behavior asfalse
: onlytrue
does something.)allowHTML
, see Add "allowHTML" option for Markdoc with HTML parsing/processing astro#7597ignoreIndentation
, see feat(markdoc): allowIndentation integration option astro#8802In
mdx
:extendMarkdownConfig
, see Markdown and MDX configuration rework astro#5684recmaPlugins
, see [MDX] SupportrecmaPlugins
config astro#5146 (note:PluggableList
is not an Astro type but aunified
one and I don't think we reexport it, but I don't have a better idea to describe the type)optimize
, see Add MDX optimize option astro#7151In
partytown
: this one is tricky because all the config options comes directly frompartytown
... so noSince
, and it's possible that the types change ifpartytown
is updated! But I still added the type for the few config options we describe based on https://github.com/QwikDev/partytown/blob/main/src/lib/types.ts#L398In
preact
:compat
, see [ci] release astro#3759In
solid-js
: I only formatted an API reference blockIn
vercel
: the missingSince
are corrects, I only formatted the API reference blocksIn
vue
:appEntrypoint
, see [Vue] add support forappEntrypoint
astro#5075jsx
: the second type is a renamed imported type, so I thinkobject
is enough since we explain below what the object is.devtools
: same, we explain the object type in "Customizing Vue DevTools" so I think this is enoughI haven't updated the other integrations guide because the API references were already there!
Related issues & labels (optional)
consistency/formatting
,improve or update documentation