Skip to content

add information about using TypeScript with layouts#8763

Merged
at-the-vr merged 9 commits into
withastro:mainfrom
at-the-vr:atv/docs/layout-ts
Jul 31, 2024
Merged

add information about using TypeScript with layouts#8763
at-the-vr merged 9 commits into
withastro:mainfrom
at-the-vr:atv/docs/layout-ts

Conversation

@at-the-vr
Copy link
Copy Markdown
Member

@at-the-vr at-the-vr commented Jul 10, 2024

Description (required)

  • Quickly guide the user how to get typesafety and autocompletion in any astro layout

Related issues & labels (optional)

@vercel

This comment was marked as outdated.

@netlify
Copy link
Copy Markdown

netlify Bot commented Jul 10, 2024

Deploy Preview for astro-docs-2 ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit ce7b540
🔍 Latest deploy log https://app.netlify.com/sites/astro-docs-2/deploys/66a9a9ca9fedff00085b668a
😎 Deploy Preview https://deploy-preview-8763--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 configuration.

@astrobot-houston
Copy link
Copy Markdown
Contributor

astrobot-houston commented Jul 10, 2024

Lunaria Status Overview

🌕 This pull request will trigger status changes.

Learn more

By default, every PR changing files present in the Lunaria configuration's files property will be considered and trigger status changes accordingly.

You can change this by adding one of the keywords present in the ignoreKeywords property in your Lunaria configuration file in the PR's title (ignoring all files) or by including a tracker directive in the merged commit's description.

Tracked Files

Locale File Note
en basics/layouts.mdx Source changed, localizations will be marked as outdated.
Warnings reference
Icon Description
🔄️ The source for this localization has been updated since the creation of this pull request, make sure all changes in the source have been applied.

Copy link
Copy Markdown
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

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

Thank you so much for running with this @at-the-vr ! I love clearing issues, and I think this is really helpful! Take a look at my comments and see what you think.

P.S. idea for another PR: the basic example, maybe we could also find a spot for something just like it on the astro components page so that where we talk about props, we show a TypeScript example there, too!

Comment thread src/content/docs/en/basics/layouts.mdx Outdated
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
Comment thread src/content/docs/en/basics/layouts.mdx Outdated
</html>
```

You can override Astro's default `Props` to handle situations where your layout recieves props of different types:
Copy link
Copy Markdown
Member Author

@at-the-vr at-the-vr Jul 10, 2024

Choose a reason for hiding this comment

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

Update: friction resolved by avoiding it in the first stage 😆

⚠️ friction alert ⚠️ You can override ... different types is incorrect afaik because Props can be defined/override to include different types as described in snippet above and I can't think of what it should be 😓
Can any experienced around TypeScript ecosystem suggest a better approach? Thanks in advance ❤️

@sarah11918 sarah11918 added the add new content Document something that is not in docs. May require testing, confirmation, or affect other pages. label Jul 10, 2024
Comment thread src/content/docs/en/basics/layouts.mdx Outdated
</html>
```

You can override Astro's default `Props` to handle situations where your layout recieves props of different types:
Copy link
Copy Markdown
Member

@sarah11918 sarah11918 Jul 19, 2024

Choose a reason for hiding this comment

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

Suggested change
You can override Astro's default `Props` to handle situations where your layout recieves props of different types:
You can also define custom `Props` to handle situations where your layout receives props of different types:

@at-the-vr , Is it just the wording that is technically not correct to say? what do you think if we just go a little more general like this? 😅

Or, is the entire example not correct?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Apologies for being blunt but the whole sentence You can override Astro's ... props of different types: happens to be incorrect with what I know of Types and Astro handling them. The suggestion is not doing any good here honestly.

Props can be custom in Astro and that's correct, but Astro.props.frontmatter is very specific. I believe its mentioned in Markdown Layout Props - Please correct me if I happen to be faulty here 😓

So when someone attempts the following (similar to OP's Issue linked)

---
// snippet from Blog Template
type Props = CollectionEntry<"blog">["data"];

const { title, description, pubDate, updatedDate, heroImage } = Astro.props.frontmatter || Astro.props;
---

They receive the Property frontmatter does not exists Error.

With respect to the Blog template this issue can be resolved by CustomProps like this:

+ type CustomProps = CollectionEntry<"blog">["data"] & {
  title: string,
  description: string,
  pubDate: Date,
  updatedDate: Date,
  heroImage: string,
}

+ const { title, description, pubDate, updatedDate, heroImage } :CustomProps = 
 Astro.props.frontmatter || Astro.props;
---

and that's what I want to address in that sentence

My direction: Here's a way you can avoid Property frontmatter does not exists error
Docs direction: Have to rely on Sarah and Team Astro for this one 🙏

@rgov
Copy link
Copy Markdown
Contributor

rgov commented Jul 29, 2024

As I commented in #8934, an overlooked aspect of this is that content entry data is processed through Zod, which can transform frontmatter entries. For example, in the blog example, the pubDate key is coerced to a Date object.

https://github.com/withastro/astro/blob/0dcef3ab171bd7f81c2f99e9366db3724aa7091b/examples/blog/src/content/config.ts#L10

I believe all frontmatter entries for a page would remain as they are parsed, as Strings or whatever basic types. My experience recently trying to use the BlogEntry.astro layout for a Markdown page was that pubDate got parsed as a String (the documentation says they're interpreted as dates when not quoted, but I didn't find that to be true), so it would not match the type definition in this comment.

Arbitrary transformations are possible through the Zod API, so it's not enough to rely on the frontmatter parser assigning types automatically, in the general case.

I'm not well-versed in TypeScript but this is the hack way I found to do it (which probably should be combined with @at-the-vr's use of CollectionEntry<"blog">["data"]?) was:

import { collections } from '../content/config';
const { author } = Astro.props.frontmatter ? collections.blog.schema.parse(Astro.props.frontmatter) : Astro.props;

It might be worth rethinking and unifying frontmatter handling between pages and content entries, which would be more conceptually ideal, rather than requiring the user either use (and know to use) hacks like applying the schema parser in their layout, or duplicating type definitions.

Comment thread src/content/docs/en/basics/layouts.mdx Outdated
@at-the-vr
Copy link
Copy Markdown
Member Author

at-the-vr commented Jul 30, 2024

quoting @rgov comment

Is the "pubDate getting passed as date" something you found in Astro Docs/Blog Repo because that should be rectified(coercion helps with Formatting for some context).
Edit: Now i see what you mean 😓 , that's true and I would coerce it as well (parsing Date with all the timestamp jargon is painful honestly)

I am afraid to tell ya, a hack is not what Docs is supposed to tell its users. Additionally I could not make it work on my end 😞 without TypeScript complaining. Trying hard not to put down your suggestion but because its working I will ask if you can document this and share for future usecases.

As for unifying frontmatter handling, perhaps someone with more experience in this area could provide insights or suggestions on how to approach this 🙌

at-the-vr and others added 2 commits July 30, 2024 20:23
- taking suggestions from Sarah

Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
Comment thread src/content/docs/en/basics/layouts.mdx Outdated
@sarah11918
Copy link
Copy Markdown
Member

Thanks for updating this, @at-the-vr ! I do think we should maybe also remove the "two layouts in one" section here. I'm not sure it's really helpful anymore. Would you be willing to remove that entire section, too? Then, when you're happy with this, I think it's good to merge!

Copy link
Copy Markdown
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

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

Approving the new section as-is, and would also approve if you decided to delete the problematic existing section, too! 😄

@rgov
Copy link
Copy Markdown
Contributor

rgov commented Jul 30, 2024

I am afraid to tell ya, a hack is not what Docs is supposed to tell its users.

I agree, but I think that the existing suggestion of Astro.props.frontmatter || Astro.props is a hack too. Astro.props (from a content entry) would be properly transformed, Astro.props.frontmatter (from a page) would not be. So I think it's incorrect for the documentation to suggest these are interchangeable/equivalent with the || operator. It may work in a few cases, but users will encounter issues even in fairly basic use cases, like the Blog example.

@at-the-vr
Copy link
Copy Markdown
Member Author

existing suggestion of Astro.props.frontmatter || Astro.props is a hack too

This is a can of worms, and clearly I lack solution for this. There is one way how Official Portfolio does it which I find fitting your usecase. If it doesn't I gotta ask you to share your thoughts in Astro Lounge to receive a much proper response. Thanks for chiming in the PR ❤️

@at-the-vr
Copy link
Copy Markdown
Member Author

!coauthor

@github-actions
Copy link
Copy Markdown

Co-authored-by: Houston (Bot) <108291165+astrobot-houston@users.noreply.github.com>
Co-authored-by: Ryan Govostes <108767+rgov@users.noreply.github.com>
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
Co-authored-by: Chris Swithinbank <357379+delucis@users.noreply.github.com>

@at-the-vr at-the-vr changed the title add information about using TypeScript for one single layout add information about using TypeScript with layouts Jul 31, 2024
@at-the-vr at-the-vr merged commit 1ebaf2c into withastro:main Jul 31, 2024
ArmandPhilippot added a commit to ArmandPhilippot/astro-docs that referenced this pull request Jul 31, 2024
* Apply changes from withastro#8763
* Reword Nesting Layouts translation
* Fix a wrong link in layouts tutorial
yanthomasdev added a commit that referenced this pull request Aug 1, 2024
* Apply changes from #8763
* Reword Nesting Layouts translation
* Fix a wrong link in layouts tutorial

Co-authored-by: Yan <61414485+yanthomasdev@users.noreply.github.com>
@sarah11918 sarah11918 added the feedback-improvement Response to widget/Discord feedback label Aug 14, 2024
trueberryless added a commit to trueberryless/withastro-docs that referenced this pull request Nov 1, 2024
jsparkdev added a commit that referenced this pull request Nov 25, 2024
* update translation #9333

* update translation #8763

* start from scratch

* finish translation

* fix links

one link is not available yet in german language

* nicer

* even nicer nicer

* Update src/content/docs/de/basics/layouts.mdx

Co-authored-by: Junseong Park <39112954+jsparkdev@users.noreply.github.com>

* remove TODO comment

* Update layouts.mdx

---------

Co-authored-by: Yan <61414485+yanthomasdev@users.noreply.github.com>
Co-authored-by: Junseong Park <39112954+jsparkdev@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

add new content Document something that is not in docs. May require testing, confirmation, or affect other pages. feedback-improvement Response to widget/Discord feedback

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Using a common Layout component for Markdown pages and content collections How do I use one layout for .md, .mdx, and .astro files with TypeScript?

5 participants