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

Update integration docs #3292

Merged
merged 5 commits into from May 20, 2023
Merged

Update integration docs #3292

merged 5 commits into from May 20, 2023

Conversation

TheOtterlord
Copy link
Member

What kind of changes does this PR include?

  • New or updated content

Description

  • Closes astro:build:done hook missing parameter #3257
  • What does this PR change?
    • Updates the integration docs to include the pages param. Seems this was removed previously as pages was due to be deprecated, but never was. routes also currently has some limitations, so pages is still useful for the time being.
    • This also removes a note about routes metadata being empty in SSR because that doesn't appear to be true (example).

@netlify
Copy link

netlify bot commented May 18, 2023

Deploy Preview for astro-docs-2 ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 5084dbf
🔍 Latest deploy log https://app.netlify.com/sites/astro-docs-2/deploys/646915f13040ec00085d9d65
😎 Deploy Preview https://deploy-preview-3292--astro-docs-2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@TheOtterlord
Copy link
Member Author

Removed the types section. Check links failed it and it's honestly not necessary. The type doesn't actually exist, let alone is anything exported to the user, and with one property it feels like a separate section just adds more scroll height more than anything.

Copy link
Member

@dreyfus92 dreyfus92 left a comment

Choose a reason for hiding this comment

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

LGTM! ✅

@@ -330,7 +330,7 @@ The address, family and port number supplied by the [NodeJS Net module](https://
**Why:** To access generated routes and assets for extension (ex. copy content into the generated `/assets` directory). If you plan to transform generated assets, we recommend exploring the [Vite Plugin API](https://vitejs.dev/guide/api-plugin.html) and [configuring via `astro:config:setup`](#updateconfig-option) instead.

```js
'astro:build:done'?: (options: { dir: URL; routes: RouteData[] }) => void | Promise<void>;
'astro:build:done'?: (options: { dir: URL; routes: RouteData[], pages: PageData[] }) => void | Promise<void>;
Copy link
Contributor

Choose a reason for hiding this comment

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

pages here should probably be typed as { pathname: string }>[] here to match Astro's types, but it'd be a great idea for us to add a PageData type into Astro's type definitions to make this more future proof!

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah yep, I missed this when removing the types. I'll fix this in a sec!

@sarah11918 sarah11918 added the improve documentation Enhance existing documentation (e.g. add an example, improve description) label May 19, 2023
@sarah11918
Copy link
Member

Alright, then let's do this thing! 🥳

@sarah11918 sarah11918 merged commit de63162 into withastro:main May 20, 2023
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improve documentation Enhance existing documentation (e.g. add an example, improve description)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

astro:build:done hook missing parameter
4 participants