feat: add @pinia-colada/sdk plugin #1680
Conversation
|
|
🦋 Changeset detectedLatest commit: 9e4cc3d The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
@josh-hemphill this is amazing! Let's figure out the details, but I'll be more than happy to merge this. I sent you a request on LinkedIn if you'd like to connect and talk about your experience writing this plugin + the details above
| _handler: handler, | ||
| exportFromIndex: true, | ||
| name: '@pinia-colada/sdk', | ||
| output: 'queries', |
There was a problem hiding this comment.
This file would then also be named colada if you agree with the change above. We might allow users to rename it one day, but for now I'd prefer consistency over customisation
packages/openapi-ts/src/plugins/@pinia-colada/sdk/__tests__/plugin.test.ts
Outdated
Show resolved
Hide resolved
| * Group queries by tag into separate files? | ||
| * @default false | ||
| */ | ||
| groupByTag?: boolean; |
There was a problem hiding this comment.
Can you explain the reasoning behind offering this functionality?
There was a problem hiding this comment.
It a preference that wasn't available with the current sdk plugin. It's very Vue in style though; it allows for a decent amount of lazy-loading and/or treeshaking if you're following the standard Vue import patterns, just importing for the contexts you need in individual Vue SFC files.
You can still import the root index file to get everything in one go, but can progressively add narrower import contexts to components to cut down on bundled component sizes.
And with Vite auto-import and type caching, a lot of the context narrowing can happen behind the scenes at build time if it has separate files to work with
There was a problem hiding this comment.
Thank you for the context! I'd like to see this in snapshot tests, it will be way easier to reason through it. We will most likely want to add this to SDKs too, it's just a matter of priority
There was a problem hiding this comment.
I'd like to see added some snapshot tests so we can see the final output + typecheck in CI
There was a problem hiding this comment.
Should I add a scenario to the openapi-ts\test\plugins.test.ts or add file snapshots to this directory, or both? Since I'm not sure the scenarios are specifically covering the behavior of this plugin, though not sure how much that matters.
There was a problem hiding this comment.
Yes please. Drop a few tests there running through various permutations of these configs, whatever you think is necessary to cover, and then pnpm openapi-ts test:update. I will likely need to refactor this again in the future, but it does the job for now
|
@josh-hemphill for the failing CI, you need to register this plugin in https://github.com/hey-api/openapi-ts/blob/2dc380eabc17c723654beb04ecd7bce6d33d3b49/packages/openapi-ts/src/plugins/index.ts and add it to types in https://github.com/hey-api/openapi-ts/blob/2dc380eabc17c723654beb04ecd7bce6d33d3b49/packages/openapi-ts/src/plugins/types.d.ts. I assume you've been importing it manually up to this point? This will make it so that it's readily available in the config. |
|
Later, I will write documentation for the plugin once we've got the API finalised and test snapshots. I will also ask you to add |
There's some room for improvement, and people may want a different sdk style or abstracted, but I thought this was a good starting point to share and get some feedback on.
Right now it tries to detect whether to wrap endpoints in
createQueryorcreateMutationbased on first overrides if they exist, then if it's aGETorDELETEendpoint it'll assumecreateQuery, and finally if there's no body schema it will fall back tocreateQuery; there's OpenAPI extension that seems like it'd be a good indicator (x-readonly), but the extensions don't seem to be available yet, so I just commented that out with a ToDo.Maybe closes #1242
closes #1159