feat(sdk): standalone package #3297
Draft
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
added a commit
that referenced
this pull request
Jan 14, 2025
## Changes Contributes to https://linear.app/nango/issue/NAN-2265/remove-shared-from-cli - Copy SDK types to `@nangohq/types` It's to prepare the field when I introduce a dedicated package for the SDK, so we can reuse types across the codebase. Those types are not used at the moment. (see #3297 for full PR) - Fix types for pagination It wasn't using a discriminated object which makes type narrowing impossible - Fix types for response saver
bodinsamuel
added a commit
that referenced
this pull request
Jan 14, 2025
## Changes Contributes to https://linear.app/nango/issue/NAN-2265/remove-shared-from-cli Split of #3297 - NangoProps use DBSyncConfig In order to stop using shared I need to stop using the types from shared, so instead of copying an invalid type I took the time to use the proper one across the board.
bodinsamuel
commented
Jan 15, 2025
packages/runner-sdk/models.d.ts
Outdated
There was a problem hiding this comment.
This is a temporary fix, until we find a way to inline all types inside a d.ts or better, get rid of models.ts altogether
That means if you modify the SDK you need to manually change this file for the next few weeks.
| @@ -151,50 +145,44 @@ describe('Pagination', () => { | |||
| token_url: '', | |||
| docs: '' | |||
| }; | |||
| (await import('@nangohq/node')).Nango.prototype.getIntegration = vi.fn().mockReturnValue({ config: { provider: 'github' } }); | |||
| vi.spyOn(providerService, 'getProvider').mockImplementation(() => provider); | |||
| vi.spyOn(await import('@nangohq/shared-public'), 'getProvider').mockImplementation(() => provider); | |||
There was a problem hiding this comment.
Since dryRun is no longer available in the backend, I had to change the mocking strategy, thankfully it was not too bad
| @@ -0,0 +1,55 @@ | |||
| import path from 'node:path'; | |||
There was a problem hiding this comment.
This is more or less, providers.ts that was in shared
bodinsamuel
added a commit
that referenced
this pull request
Jan 16, 2025
## Changes Contributes to https://linear.app/nango/issue/NAN-2265/remove-shared-from-cli Split of #3297 - New package `@nangohq/providers` Only contains providers.yaml. Compiled and included in tooling but not used in code yet
bodinsamuel
added a commit
that referenced
this pull request
Jan 16, 2025
## Changes Contributes to https://linear.app/nango/issue/NAN-2265/remove-shared-from-cli Split of #3297 - **New package `@nangohq/runner-sdk`** It contains the shared SDK, data validation, and paginate service. The SDK is now an abstract class. It includes the common logic, but if a method is different between the CLI and Runner, it is not implemented. The code is mostly similar, except for: - Replaced NangoError by SDKError - Removed all the backend logic (e.g: proxy, instrumentation, persist, etc.) - Hardcoded `models.ts` It can't be computed like it used to (it wasn't really working anyway), it's hardcoded and delivered as-is for the CLI, so it needs to be maintained manually. We will change that in the coming weeks.
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.
Context
We need to stop using
sharedin the CLI because it contains a lot (e.g: unrelated database models, internal packages, unrelated npm packages, etc.). We imported shared because it contains the runner SDK which is used in the CLI and in the runner +providers.yaml.The goal of this project is stop importing shared but still be able to share some code between CLI and Runner.
Changes
New package
@nangohq/runner-sdkIt contains the shared SDK, data validation, and paginate service. The SDK is now an abstract class. It includes the common logic, but if a method is different between the CLI and Runner, it is not implemented.
The code is mostly similar, except for:
New package
@nangohq/shared-publicThis package contains logic shared between the CLI, Runner SDK and Backend. And also
providers.yaml!It's relatively small for now, the goal is to limit this as much as we can, so it doesn't end up like
shared.I could not find a good name. I often use
coreorcommonbut there would be no difference withutilswhich could create some confusion. Happy to change if needed.CLI and Runner now implement their own SDK
Using the new package, they implement the custom part (what was inside
sync.ts). The footprint of duplicated code is small right now so I feel like it works but happy to discuss this choice.Simplify npm publish
Since we don't need to pack things up anymore, it's faster and simpler (but still takes a bit of time)
Todo