Skip to content

Add getEnvironment to BaseClient #15488

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Feb 4, 2025
Merged

Add getEnvironment to BaseClient #15488

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
2bdb6b5
Add getEnvironment to BaseClient
bzwrk Feb 3, 2025
13c76b1
fix not running tests
tjk Feb 3, 2025
d0ac161
try this
tjk Feb 3, 2025
642295e
jest and type stuff
tjk Feb 4, 2025
e2af32e
Fix linter errors
bzwrk Feb 4, 2025
9d77584
Merge branch 'master' into biz/dj-2857-add-getenvironment-to-sdk-base…
bzwrk Feb 4, 2025
98af366
Fix type error
bzwrk Feb 4, 2025
69e1260
Update pnpm-lock.yaml
bzwrk Feb 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions packages/sdk/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@

# Changelog

## [1.3.2] - 2025-02-3

### Changed

- Add getEnvironment function to BaseClient

## [1.3.1] - 2025-01-30

### Changed
Expand Down
5 changes: 4 additions & 1 deletion packages/sdk/jest.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
export default {
preset: "ts-jest",
testEnvironment: "node",
roots: [
"<rootDir>/src",
Expand All @@ -15,13 +14,17 @@ export default {
moduleNameMapper: {
"^(.+)\\.js$": "$1",
},
extensionsToTreatAsEsm: [
".ts",
],
transform: {
// '^.+\\.[tj]sx?$' to process ts,js,tsx,jsx with `ts-jest`
// '^.+\\.m?[tj]sx?$' to process ts,js,tsx,jsx,mts,mjs,mtsx,mjsx with `ts-jest`
"^.+\\.[jt]sx?$": [
"ts-jest",
{
tsconfig: "tsconfig.node.json",
useESM: true,
},
],
},
Expand Down
4 changes: 2 additions & 2 deletions packages/sdk/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "@pipedream/sdk",
"type": "module",
"version": "1.3.1",
"version": "1.3.2",
"description": "Pipedream SDK",
"main": "./dist/server.js",
"module": "./dist/server.js",
Expand Down Expand Up @@ -40,7 +40,7 @@
"prepublish": "pnpm run build",
"prebuild": "node scripts/updateVersion.js",
"build": "rm -rf dist && pnpm run prebuild && tsup --config tsup.server.cjs.config.js && tsup --config tsup.server.esm.config.js && tsup --config tsup.browser.config.js",
"test": "node --experimental-vm-modules node_modules/.bin/jest",
"test": "NODE_OPTIONS=\"$NODE_OPTIONS --experimental-vm-modules\" jest",
"watch": "nodemon --watch src --exec 'pnpm run build'",
"cli": "node dist/server/cli.js"
},
Expand Down
16 changes: 11 additions & 5 deletions packages/sdk/src/server/__tests__/server.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { jest } from "@jest/globals"
import {
BackendClient,
BackendClientOpts,
Expand Down Expand Up @@ -50,7 +51,7 @@
url: "https://api.pipedream.com/v1/test-path",
},
response: {
json: { data: "test-response" },

Check failure on line 54 in packages/sdk/src/server/__tests__/server.test.ts

View workflow job for this annotation

GitHub Actions / Lint Code Base 

Expected a line break after this opening brace

Check failure on line 54 in packages/sdk/src/server/__tests__/server.test.ts

View workflow job for this annotation

GitHub Actions / Lint Code Base 

Expected a line break before this closing brace
},
})

Expand All @@ -72,7 +73,7 @@
},
},
response: {
json: { success: true },

Check failure on line 76 in packages/sdk/src/server/__tests__/server.test.ts

View workflow job for this annotation

GitHub Actions / Lint Code Base 

Expected a line break after this opening brace

Check failure on line 76 in packages/sdk/src/server/__tests__/server.test.ts

View workflow job for this annotation

GitHub Actions / Lint Code Base 

Expected a line break before this closing brace
},
})

Expand Down Expand Up @@ -116,7 +117,7 @@
},
},
response: {
json: { success: true },

Check failure on line 120 in packages/sdk/src/server/__tests__/server.test.ts

View workflow job for this annotation

GitHub Actions / Lint Code Base 

Expected a line break after this opening brace

Check failure on line 120 in packages/sdk/src/server/__tests__/server.test.ts

View workflow job for this annotation

GitHub Actions / Lint Code Base 

Expected a line break before this closing brace
},
})

Expand Down Expand Up @@ -144,7 +145,7 @@
},
},
response: {
json: { success: true },

Check failure on line 148 in packages/sdk/src/server/__tests__/server.test.ts

View workflow job for this annotation

GitHub Actions / Lint Code Base 

Expected a line break after this opening brace

Check failure on line 148 in packages/sdk/src/server/__tests__/server.test.ts

View workflow job for this annotation

GitHub Actions / Lint Code Base 

Expected a line break before this closing brace
},
})

Expand Down Expand Up @@ -311,10 +312,10 @@
url: `https://api.pipedream.com/v1/connect/${projectId}/accounts?app=app-1`,
},
response: {
json: [{

Check failure on line 315 in packages/sdk/src/server/__tests__/server.test.ts

View workflow job for this annotation

GitHub Actions / Lint Code Base 

A linebreak is required after '['
id: "account-1",
name: "Test Account",
}],

Check failure on line 318 in packages/sdk/src/server/__tests__/server.test.ts

View workflow job for this annotation

GitHub Actions / Lint Code Base 

A linebreak is required before ']'
},
});

Expand Down Expand Up @@ -360,7 +361,7 @@
});

describe("deleteAccount", () => {
it("should delete a specific account by ID", async () => {

Check warning on line 364 in packages/sdk/src/server/__tests__/server.test.ts

View workflow job for this annotation

GitHub Actions / Lint Code Base 

Test has no assertions
fetchMock.expectAccessTokenSuccess();
fetchMock.expect({
request: {
Expand All @@ -377,7 +378,7 @@
});

describe("deleteAccountsByApp", () => {
it("should delete all accounts associated with a specific app", async () => {

Check warning on line 381 in packages/sdk/src/server/__tests__/server.test.ts

View workflow job for this annotation

GitHub Actions / Lint Code Base 

Test has no assertions
fetchMock.expectAccessTokenSuccess();
fetchMock.expect({
request: {
Expand All @@ -394,7 +395,7 @@
});

describe("deleteExternalUser", () => {
it("should delete all accounts associated with a specific external ID", async () => {

Check warning on line 398 in packages/sdk/src/server/__tests__/server.test.ts

View workflow job for this annotation

GitHub Actions / Lint Code Base 

Test has no assertions
fetchMock.expectAccessTokenSuccess();
fetchMock.expect({
request: {
Expand Down Expand Up @@ -726,17 +727,22 @@

beforeEach(() => {
intercepts = [];
jest.spyOn(global, "fetch").mockImplementation(jest.fn((url: string, init: RequestInit) => {
// without these generics this fails typecheck and can't figure out why
jest.spyOn<any, any, any>(global, "fetch").mockImplementation(jest.fn<typeof fetch>(async (...args: Parameters<typeof fetch>) => {
const [url, init] = args
let json: any
if (init.body && typeof init.body === "string") {
if (init?.body && typeof init.body === "string") {
try {
json = JSON.parse(init.body)
} catch {}
}
if (url instanceof Request) {
throw new Error("not supported")
}
const ifOpts: IfOpts = {
method: init.method || "GET",
url,
headers: init.headers as Record<string, string> || {},
method: init?.method || "GET",
url: url.toString(),
headers: init?.headers as Record<string, string> || {},
json,
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve type safety and error handling.

Several improvements can be made to enhance type safety and error handling:

  1. Replace any types with specific types
  2. Add error handling for the empty catch block
  3. Fix array bracket formatting

Apply this diff to improve the code:

-    jest.spyOn<any, any, any>(global, "fetch").mockImplementation(jest.fn<typeof fetch>(async (...args: Parameters<typeof fetch>) => {
-      const [url, init] = args
+    jest.spyOn(global, "fetch").mockImplementation(jest.fn(async (url: string | URL | Request, init?: RequestInit) => {
       let json: any
-      if (init?.body && typeof init.body === "string") {
+      if (init?.body && typeof init.body === "string") {
         try {
           json = JSON.parse(init.body)
-        } catch {}
+        } catch (error) {
+          console.warn("Failed to parse request body:", error)
+        }
       }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
jest.spyOn<any, any, any>(global, "fetch").mockImplementation(jest.fn<typeof fetch>(async (...args: Parameters<typeof fetch>) => {
const [url, init] = args
let json: any
if (init.body && typeof init.body === "string") {
if (init?.body && typeof init.body === "string") {
try {
json = JSON.parse(init.body)
} catch {}
}
if (url instanceof Request) {
throw new Error("not supported")
}
const ifOpts: IfOpts = {
method: init.method || "GET",
url,
headers: init.headers as Record<string, string> || {},
method: init?.method || "GET",
url: url.toString(),
headers: init?.headers as Record<string, string> || {},
json,
jest.spyOn(global, "fetch").mockImplementation(jest.fn(async (url: string | URL | Request, init?: RequestInit) => {
let json: any
if (init?.body && typeof init.body === "string") {
try {
json = JSON.parse(init.body)
} catch (error) {
console.warn("Failed to parse request body:", error)
}
}
if (url instanceof Request) {
throw new Error("not supported")
}
const ifOpts: IfOpts = {
method: init?.method || "GET",
url: url.toString(),
headers: init?.headers as Record<string, string> || {},
json,
🧰 Tools
🪛 ESLint

[error] 731-731: Unexpected any. Specify a different type.

(@typescript-eslint/no-explicit-any)

[error] 731-731: Unexpected any. Specify a different type.

(@typescript-eslint/no-explicit-any)

[error] 731-731: Unexpected any. Specify a different type.

(@typescript-eslint/no-explicit-any)

[error] 732-732: A linebreak is required after '['.

(array-bracket-newline)

[error] 732-732: There should be a linebreak after this element.

(array-element-newline)

[error] 732-732: A linebreak is required before ']'.

(array-bracket-newline)

[error] 733-733: Unexpected any. Specify a different type.

(@typescript-eslint/no-explicit-any)

[error] 737-737: Empty block statement.

(no-empty)

}
for (let i = 0; i < intercepts.length; i++) {
Expand Down
2 changes: 1 addition & 1 deletion packages/sdk/src/server/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ export class BackendClient extends BaseClient {
token: string
expiresAt: number
};
protected override projectId: string;
protected override projectId: string = "";
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Reconsider adding a default empty string value for projectId.

The default empty string value for projectId seems unnecessary and potentially problematic because:

  1. projectId is a required field in BackendClientOpts (line 44)
  2. The constructor always sets it from opts.projectId (line 158)
  3. An empty projectId might cause issues in API requests

Remove the default value since it's not needed:

-  protected override projectId: string = "";
+  protected override projectId: string;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
protected override projectId: string = "";
protected override projectId: string;


/**
* Constructs a new ServerClient instance.
Expand Down
8 changes: 8 additions & 0 deletions packages/sdk/src/shared/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -871,6 +871,14 @@ export abstract class BaseClient {
this.workflowDomain = workflowDomain;
}

/**
* Retrieves the current environment the client is configured to use.
* @returns {string} The current environment.
*/
public getEnvironment(): string {
return this.environment;
}

/**
* Makes an HTTP request
*
Expand Down
2 changes: 0 additions & 2 deletions pnpm-lock.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading