feat(sdk): add new endpoints #2747
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bodinsamuel
changed the title
TBonnin
reviewed
Sep 19, 2024
docs-v2/reference/sdks/node.mdx
Outdated
| @@ -94,6 +157,42 @@ await nango.listIntegrations() | |||
|
|
|||
| Returns a specific integration. | |||
|
|
|||
| ```js | |||
| await nango.getIntegrationV2(<UNIQUE_KEY>, { include?: ['webhook'] }); | |||
There was a problem hiding this comment.
what about having the new method take an single options object instead, then signature is different and we can use method overloading.
type getIntegrationOptions = {uniqueKey: string, query?: ...}
public async getIntegration({uniqueKey, query} : getIntegrationOptions);
public async getIntegration(name: string);
public async getIntegration(nameOrOptions: string | getIntegrationOptions) {
...
Same for getProvider({name}: { name: string}) to keep it consistent
There was a problem hiding this comment.
Could work indeed, it's a paradigm shift though. Usually in Nodejs SDK the first param to get() is an id, string or number not an object.
We can have naming consistency:
- getProvider({ id })
- getIntegration({ id })
- getConnection({ id })or still use the name
- getProvider({ name })
- getIntegration({ unique_key })
- getConnection({ connection_id })wdyt?
There was a problem hiding this comment.
I prefer option 2. It is a bit more self-explanatory what the param is.
packages/server/lib/controllers/integrations/uniqueKey/getIntegration.ts
Show resolved
Hide resolved
khaliqgant
reviewed
Sep 20, 2024
| @@ -4,7 +4,7 @@ const args = process.argv.slice(2); | |||
| const nango = new Nango({ host: 'http://localhost:3003', secretKey: args[0] }); | |||
|
|
|||
| nango | |||
| .getIntegration(args[1], args[2]) | |||
| .getIntegrationV2(args[1]) | |||
There was a problem hiding this comment.
Would rather this be a separate file until we actually retire the original method
khaliqgant
approved these changes
Sep 20, 2024
|
@TBonnin Updated with signature change, thanks for the suggestion 💪🏻 |
TBonnin
approved these changes
Sep 20, 2024
hassan254-prog
pushed a commit
that referenced
this pull request
Sep 25, 2024
## Describe your changes Fixes https://linear.app/nango/issue/NAN-1753/document-new-endpoints-integrations-and-providers - Add new endpoints to the SDK There is only one method that was different enough, queries and output that even with signature overload it wasn't possible to make it non-breaking. Honestly, I didn't find anything compelling to deprecate without either being ugly or breaking customers' scripts, so very open to suggestions. - Document new SDK method - Add support for `include=webhook` in `GET /integrations/:key` The SDK needs it, and some customers use it. It's a secret but not credentials, so I didn't put a too vague name. If we need credentials at some point, we can differentiate.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Describe your changes
Fixes https://linear.app/nango/issue/NAN-1753/document-new-endpoints-integrations-and-providers
Add new endpoints to the SDK
There is only one method that was different enough, queries and output that even with signature overload it wasn't possible to make it non-breaking. Honestly, I didn't find anything compelling to deprecate without either being ugly or breaking customers' scripts, so very open to suggestions.
Document new SDK method
Add support for
include=webhookinGET /integrations/:keyThe SDK needs it, and some customers use it. It's a secret but not credentials, so I didn't put a too vague name. If we need credentials at some point, we can differentiate.