-
Notifications
You must be signed in to change notification settings - Fork 81
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
feat(module): handle v5
#424
Conversation
✅ Deploy Preview for nuxt-strapi-module canceled.
|
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.
Thanks for looking into this!
- You can remove the unnecessary
package-lock.json
as we're usingpnpm
- You don't need to split the composable for
v5
, you can just have auseStrapi
, this was implemented like this to prevent breaking changes.
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.
You might also need to export the v5
types: https://github.com/nuxt-modules/strapi/blob/dev/src/runtime/types/index.d.ts#L574
So updated according to your comments. |
|
||
interface StrapiV5Client<T> { | ||
find<F = T>(contentType: string, params?: Strapi5RequestParams): Promise<Strapi5ResponseMany<F>> | ||
findOne<F = T>(contentType: string, docuemntId?: string | Strapi5RequestParams, params?: Strapi5RequestParams): Promise<Strapi5ResponseSingle<F>> |
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.
Why not keep the old signature here? The id?: string | number | Strapi5RequestParams
seems fine to me.
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.
changed it to documentId (don't mind the typo, will fix that) because Strapi v5 is moving away from using id as name and it is now documentId instead.
I wanted to reflect that change here as well. To indicate it want a documentId and not just an id.
src/runtime/types/v5.d.ts
Outdated
locale?: StrapiLocale | ||
} | ||
|
||
export type Strapi5ResponseData<T> = Strapi5SystemFields & T |
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.
Not sure you want to remove the Strapi5Response
as it might be used by users.
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.
Removed it as it was duplicate from Strapi5ResponseSingle
I"m merging this so people can try it out using the Edge package but I'd like some feedback before releasing it officially 😊 |
Hey. Thanks for the work guys. I use the old version of this on v5 of strapi. It still worked well lol |
Resolves #423
Types of changes
Description
a very basic v5 implementation. If I read their docs correctly only the returned format seems to have changed so I added that in a new v5 composable
It does needs to be tested however