feat(oas-to-snippet): plugin architecture #834
Conversation
| @@ -24,41 +24,62 @@ import petstore from './petstore.json'; | |||
| const apiDefinition = new Oas(petstore); | |||
| const operation = apiDefinition.operation('/pets', 'get'); | |||
|
|
|||
| // This is a keyed object containing formData for your operation. Available keys are: path, | |||
There was a problem hiding this comment.
Trimmed these Markdown code snippet lines down to 100 character lengths because 120 looks like shit on small screens on NPM and Github.
There was a problem hiding this comment.
fwiw i don't think this is a huge deal since both npm and github support side-scrolling on these code samples and even 100 characters is still too wide for certain screens like mobile etc.
| httpclient: { name: 'HttpClient' }, | ||
| restsharp: { | ||
| name: 'RestSharp', | ||
| install: 'dotnet add package RestSharp', |
There was a problem hiding this comment.
This supportedLanguages config now lives at the DEFAULT_LANGAUGES config in languages.ts and is largely the same except for two changes:
apiis no longer listed here undernodebecause that's been offloaded to ourhttpsnippet-client-apiplugin.installhas moved directly into@readme/httpsnippetclient configurations so that information is as close to the snippet generator as possible.
| let language: TargetId; | ||
| let target: ClientId; | ||
|
|
||
| switch (lang) { |
There was a problem hiding this comment.
this looks so much cleaner than that big if statement!
There was a problem hiding this comment.
a few initial questions, i might set up a proper sandbox environment to test this out with @darrenyong later today!
| // Our `httpsnippet-client-api` plugin uses these options so we need to pass them along. | ||
| if (plugin.target === 'node' && plugin.client.info.key === 'api') { | ||
| targetOpts.api = { | ||
| definition: oas ? oas.getDefinition() : null, | ||
| identifier: opts?.openapi?.variableName, | ||
| registryURI: opts?.openapi?.registryIdentifier, | ||
| }; | ||
| } |
There was a problem hiding this comment.
maybe i'm misunderstanding how all of this fits together, but i feel like this logic (and its corresponding opts) should be in the plugin code, not here
There was a problem hiding this comment.
it is, but these options need to get passed along from here into the plugin and the plugin wants these options as an api object. im not really sure on the best way to handle a plugin that has its own options like this other than this thing i've got here.
| * | ||
| * @example @developers/v2.0#17273l2glm9fq4l5 | ||
| */ | ||
| registryIdentifier?: string, |
There was a problem hiding this comment.
i imagine other plugins might want custom installation steps similar to api does, could we make this (and its corresponding JSDocs) more generic (i.e., call it packageName)?
There was a problem hiding this comment.
this work is a little confusing but the registryIdentifier here is the npx api install <registry identifier> part of things and variableName is the eventual post-installation package name (as that ties into the custom SDK variable name support).
There was a problem hiding this comment.
could maybe have the config for the plugin supplied alongside the plugin but that's going to require a lot of work to httpsnippet as it doesn't support options like this
const snippet = await oasToSnippet(
petstore,
petstore.operation('/user/login', 'get'),
formData,
auth,
['node', 'api'],
{
plugins: [httpsnippetClientAPIPlugin({
api: {
registryIdentifier: '@petstore/v2.0#17273l2glm9fq4l5',
}
})],
},
);
Co-authored-by: Kanad Gupta <[email protected]>
🧰 Changes
This pulls in the new plugin architecture I added to
@readme/httpsnippetin readmeio/httpsnippet#206 so we can now do a couple great things:httpsnippet-client-apidepending onoasand being used here has historically made bumping majoroasreleases a lot more problematic than it should be.apisnippets will no longer be natively supported by this library without thehttpsnippet-client-apiplugin for it.httpsnippet-plugin-companynameand we could generate snippets with a new[node, companyname]language 🧙 like magic 🪄I've also refactored our
/supportedLanguagesexport to now be agetSupportedLanguagesfunction we'll call to retrieve the languages we support. Had to make this change in order to load custom plugins into the supported languages configuration and make them be available as a supported language. 🐔 🥚I don't have any docs written on the plugin system but will write those at some point before this work goes live in our API Explorer.