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

[@astrojs/rss] Quality-of-Life Improvement to items property-related error #9299

Merged
merged 4 commits into from
Dec 6, 2023

Conversation

cdvillard
Copy link
Contributor

Changes

This commit addresses a quality-of-life concern when setting up a RSS feed using collections. Specifically:

  • it provides more context to the error message thrown when the object passed to the items property is missing any of the three required keys or if one of those keys is mistyped.
  • it adds a link to provide additional context and instruction

Testing

I tested this change similarly to how I encountered #9171. Using the examples/blog project, I simply messed around with the properties of items to ensure an error would render, and that my changes would build and reflect appropriately.

Before:
Screenshot of items error before my changes

After:
Screenshot of items error after my changes

Docs

This shouldn't necessitate any changes to the docs as this way entirely done to help improve the error solely. Usage of the package remains unchanged.

… feed

when using collections. Specifically, it provides more context to the error
message thrown when the object passed to the `items` property is missing any
of the three required keys or if one of those keys is mistyped.
Copy link

changeset-bot bot commented Dec 5, 2023

🦋 Changeset detected

Latest commit: 2b3a471

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added the docs pr A PR that includes documentation for review label Dec 5, 2023
@cdvillard
Copy link
Contributor Author

Just to put it out there, I'm not married to the copy language used, nor the method. I was a little confused by the use of errorMap on line 105. I tried to define a new conditional there to catch invalid_union types, but despite building fine, it never seemed to catch it. I have no deep experience with Zod, so I'm sure it could be something specific to the rssOptionsValidator ZodObject alone.

Copy link
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.

A docs review only: the proposed error message is fantastic. Thank you for the care and attention to detail @cdvillard! Massaged the changeset a little, so see what you think about that.

.changeset/thirty-hairs-pump.md Outdated Show resolved Hide resolved
cdvillard and others added 2 commits December 5, 2023 11:51
@sarah11918 suggested a change to the verbiage that properly formatted the update detail in question. Accepting the suggestion.

Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
@ematipico ematipico merged commit edfae50 into withastro:main Dec 6, 2023
13 checks passed
@astrobot-houston astrobot-houston mentioned this pull request Dec 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs pr A PR that includes documentation for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants