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

feat(module): handle v5 #424

Merged
merged 11 commits into from
Sep 26, 2024
Merged

feat(module): handle v5 #424

merged 11 commits into from
Sep 26, 2024

Conversation

XanderD99
Copy link
Contributor

@XanderD99 XanderD99 commented Sep 19, 2024

Resolves #423

Types of changes

  • Bug fix (a non-breaking change which fixes an issue)
  • New feature (a non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

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

Copy link

netlify bot commented Sep 19, 2024

Deploy Preview for nuxt-strapi-module canceled.

Name Link
🔨 Latest commit a5c2385
🔍 Latest deploy log https://app.netlify.com/sites/nuxt-strapi-module/deploys/66f583808670c90008939df9

Copy link
Member

@benjamincanac benjamincanac left a 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!

  1. You can remove the unnecessary package-lock.json as we're using pnpm
  2. You don't need to split the composable for v5, you can just have a useStrapi, this was implemented like this to prevent breaking changes.

Copy link
Member

@benjamincanac benjamincanac left a comment

Choose a reason for hiding this comment

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

@XanderD99
Copy link
Contributor Author

So updated according to your comments.
Added the types, as well as remove duplicates that remained the same from v4 => v5
Delete the package-lock and also added it to gitignore so it won't be added again in the future (I don't use pnpm :( )
Updated the split implementation (hope what I did was what you ment). But also updated some variable names, for example id => documentId to reflect the changed of strapi v5

src/runtime/composables-v5/useStrapi.ts Outdated Show resolved Hide resolved

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

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.

Copy link
Contributor Author

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.

locale?: StrapiLocale
}

export type Strapi5ResponseData<T> = Strapi5SystemFields & T
Copy link
Member

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.

Copy link
Contributor Author

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

@benjamincanac benjamincanac changed the title add basic v5 feat(module): handle v5 Sep 20, 2024
@XanderD99 XanderD99 mentioned this pull request Sep 23, 2024
@benjamincanac benjamincanac merged commit b8949e5 into nuxt-modules:dev Sep 26, 2024
5 checks passed
@benjamincanac
Copy link
Member

I"m merging this so people can try it out using the Edge package but I'd like some feedback before releasing it officially 😊

@BayBreezy
Copy link
Contributor

Hey. Thanks for the work guys. I use the old version of this on v5 of strapi. It still worked well lol
I usually just use a custom client with the useStrapiClient composable instead of the find, update methods that the package provide. I will give this a shot soon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Strapi v5 support
3 participants