-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Rename getEntry to getEntryBySlug #5893
Conversation
🦋 Changeset detectedLatest commit: aa395e4 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 |
Going to change this to return |
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.
Just some type cleanup. Some entry
guing changes!
Co-authored-by: Ben Holmes <[email protected]>
entryKey: E | ||
): Promise<(typeof entryMap)[C][E] & Render>; | ||
// Note that this has to accept a regular string too, for SSR | ||
entrySlug: ValidEntrySlug<C> | string |
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.
Make sure completions still work here. Doing literal unions | string
typically break the completions because TypeScript will only see string
You have to do a work around for it to keep the completions, aka | (string & {})
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 ok, thank you. I realized that the SSR use-case you're going to be grabbing a string from Astro.params so not allowing any strings would give bad type messages. I'll try your suggestion.
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.
Ok, updated here: d41cc2f
This works in my testing. If you provide a known slug value you get back just an entry. If you provide any other string you get back an entry or undefined, which is what we want.
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.
LGTM on the API! Thanks for getting this across the line in time for 2.0.
@bholmesdev I'm happy to factor these changes into my docs PR, so no action needed on your part for docs other than giving a review if you can. cc @sarah11918 so she's aware!
Changes
getEntry
togetEntryBySlug
.string
is a blocker here. Happy to be proven otherwise.Testing
Docs