Skip to content
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.

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

fix: bring eslint + TS issues to zero #1013

Merged
merged 17 commits into from
Dec 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion .github/workflows/lint_js.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ jobs:
run: pnpm install --frozen-lockfile

- name: Run ESLint
run: pnpm run eslint
run: pnpm run lint
continue-on-error: false

- name: Run Prettier
Expand Down
5 changes: 3 additions & 2 deletions js/.prettierrc
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,6 @@
"useTabs": false,
"embeddedLanguageFormatting": "auto",
"vueIndentScriptAndStyle": false,
"experimentalTernaries": false
}
"experimentalTernaries": false,
"plugins": ["prettier-plugin-organize-imports"]
}
2 changes: 1 addition & 1 deletion js/eslint.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export default [
"no-var": "warn",
"prefer-const": "warn",
"no-console": "warn",
"@typescript-eslint/no-explicit-any": "warn",
"@typescript-eslint/no-explicit-any": "error",
"@typescript-eslint/no-unused-vars": [
"warn",
{
Comment on lines 26 to 32

Choose a reason for hiding this comment

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

⚠️ Potential Issue:

Evaluate Impact of Linting Rule Change
The change from 'warn' to 'error' for the @typescript-eslint/no-explicit-any rule increases the strictness of the linting configuration. This could lead to build failures or prevent code commits if the 'any' type is used.

  • Considerations:
    • Evaluate if this aligns with the team's coding standards.
    • Assess the potential need for refactoring existing code to comply with this stricter rule.
    • Ensure the team is aware of this change to avoid unexpected disruptions in development workflows.

📌 Recommendation: If the team is not ready for this level of strictness, consider keeping it as a 'warn' and gradually refactor the codebase to reduce 'any' usage before enforcing it as an 'error'.


himanshu-dixit marked this conversation as resolved.
Show resolved Hide resolved
himanshu-dixit marked this conversation as resolved.
Show resolved Hide resolved
himanshu-dixit marked this conversation as resolved.
Show resolved Hide resolved
himanshu-dixit marked this conversation as resolved.
Show resolved Hide resolved
himanshu-dixit marked this conversation as resolved.
Show resolved Hide resolved
Comment on lines 26 to 32

Choose a reason for hiding this comment

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

🔨 Refactor:

Increase in Linting Strictness for '@typescript-eslint/no-explicit-any'
The change from 'warn' to 'error' for the @typescript-eslint/no-explicit-any rule increases the strictness of the linting process. This can lead to build failures if the codebase contains instances of any.

Actionable Steps:

  • Audit the Codebase: Conduct a thorough review of the codebase to identify all instances where any is used.
  • Refactor Code: Replace any with more specific types to ensure type safety and maintainability.
  • Test Thoroughly: After refactoring, run the full test suite to ensure that the changes do not introduce any regressions.
  • Team Communication: Inform the development team about this change to prevent unexpected build failures and to encourage best practices in type usage.

This change is beneficial for long-term code quality but requires careful implementation to avoid disrupting the development workflow. 🚀


Expand Down
4 changes: 2 additions & 2 deletions js/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,10 @@
"openapispec:generate": "npx @hey-api/openapi-ts",
"run:cli": "ts-node src/cli/index.ts",
"run:sample": "ts-node sample.ts",
"eslint": "eslint 'src/**/*.{ts,js}' --max-warnings=261",
"prettier": "prettier --write 'src/**/*.{ts,js,cjs}'",
"prettier:check": "prettier --check 'src/**/*.{ts,js,mjs,cjs}'",
"build": "rollup -c rollup.config.mjs && ./setup_cli.sh",
"lint": "eslint 'src/**/*.ts'",
"lint": "eslint 'src/**/*.{ts,js}' --ignore-pattern 'src/sdk/client/**'",
"format": "pnpm lint && pnpm prettier"
},
"bin": {
Expand Down Expand Up @@ -75,6 +74,7 @@
"open": "^8.4.0",
"openai": "^4.50.0",
"prettier": "^3.4.2",
"prettier-plugin-organize-imports": "^4.1.0",
"pusher-js": "8.4.0-rc2",
"regenerator-runtime": "^0.14.1",
"resolve-package-path": "^4.0.3",
Expand Down
18 changes: 18 additions & 0 deletions js/pnpm-lock.yaml

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

2 changes: 1 addition & 1 deletion js/prettier.config.mjs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const config = {
plugins: [],
plugins: ['prettier-plugin-organize-imports'],
semi: true,
printWidth: 140,
tabWidth: 4,
Expand Down
3 changes: 2 additions & 1 deletion js/src/cli/actions.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
/* eslint-disable no-console */
import chalk from "chalk";
import { Command } from "commander";
import { ListActionsV2Data } from "../sdk/client";
import client from "../sdk/client/client";
import { getOpenAPIClient } from "../sdk/utils/config";
import { ListActionsV2Data } from "../sdk/client";

export default class ActionCommand {
private program: Command;
Expand Down
111 changes: 65 additions & 46 deletions js/src/cli/add.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,36 @@
/* eslint-disable no-console */
import chalk from "chalk";
import { Command } from "commander";
import { Composio } from "../sdk";
import inquirer from "inquirer";
import open from "open";
import { Composio } from "../sdk";
import { GetConnectorInfoResDTO, GetConnectorListResDTO } from "../sdk/client";

type TInputField = {
name: string;
displayName?: string;
display_name?: string;
expected_from_customer?: boolean;
required?: boolean;
message_name?: string;
type?: string;
};

type THandleActionOptions = {
force?: boolean;
skipDefaultConnector?: boolean;
noBrowser?: boolean;
integrationId?: string;
authMode?: string;
scope?: string[];
label?: string[];
};

type TAuthScheme = {
auth_mode: string;
fields: TInputField[];
};

export default class AddCommand {
private program: Command;

Expand Down Expand Up @@ -42,15 +68,7 @@ export default class AddCommand {

private async handleAction(
appName: string,
options: {
force?: boolean;
skipDefaultConnector?: boolean;
noBrowser?: boolean;
integrationId?: string;
authMode?: string;
scope?: string[];
label?: string[];
}
options: THandleActionOptions
): Promise<void> {
const composioClient = new Composio({});
let integration:
Expand Down Expand Up @@ -158,25 +176,25 @@ export default class AddCommand {

private async setupConnections(
integrationId: string,
options: any
options: Record<string, unknown>
): Promise<void> {
const composioClient = new Composio({});
const data = await composioClient.integrations.get({ integrationId });
const { expectedInputFields } = data!;

const config = await this.collectInputFields(
expectedInputFields as any,
expectedInputFields as unknown as TInputField[],
true
);

if (options.scope) {
config.scopes = options.scope.join(",");
config.scopes = (options.scope as string[]).join(",");
}

const connectionData = await composioClient.connectedAccounts.create({
integrationId,
data: config,
labels: options.label,
labels: options.label as string[],
});

if (connectionData.connectionStatus === "ACTIVE") {
Expand Down Expand Up @@ -216,7 +234,7 @@ export default class AddCommand {
appName: string,
skipDefaultConnectorAuth: boolean = false,
userAuthMode?: string,
options?: any
options?: THandleActionOptions
) {
const composioClient = new Composio({});
const app = await composioClient.apps.get({
Expand All @@ -234,7 +252,7 @@ export default class AddCommand {

const testConnectors = app.testConnectors || [];

const config: Record<string, any> = {};
const config: Record<string, unknown> = {};
let useComposioAuth = true;
const authSchemeExpectOauth = ["bearer_token", "api_key", "basic"];
if (
Expand Down Expand Up @@ -273,8 +291,8 @@ export default class AddCommand {
(app.auth_schemes[0]?.auth_mode as string | undefined));

const authModes = (app.auth_schemes || []).reduce(
(acc: Record<string, any>, scheme: any) => {
acc[scheme.auth_mode] = scheme;
(acc, scheme: Record<string, unknown>) => {
acc[scheme.auth_mode as string] = scheme;
return acc;
},
{}
Expand All @@ -300,7 +318,7 @@ export default class AddCommand {
return this.handleBasicAuth(
app,
selectedAuthMode,
selectedAuthScheme,
selectedAuthScheme as TAuthScheme,
config,
integrationName
);
Expand All @@ -309,27 +327,27 @@ export default class AddCommand {
return this.handleOAuth(
app,
selectedAuthMode,
selectedAuthScheme,
selectedAuthScheme as TAuthScheme,
config,
integrationName,
options.noBrowser,
options.scope,
options?.noBrowser ?? false,
options?.scope ?? [],
useComposioAuth
);
}

private async handleBasicAuth(
app: any,
app: Record<string, unknown>,
authMode: string,
authScheme: any,
config: Record<string, any>,
authScheme: TAuthScheme,
config: Record<string, unknown>,
integrationName: string
) {
const composioClient = new Composio({});
const authConfig = await this.collectInputFields(authScheme.fields);

const integration = await composioClient.integrations.create({
appId: app.appId,
appId: app.appId as string,
authScheme: authMode,
useComposioAuth: false,
name: integrationName,
Expand All @@ -340,33 +358,39 @@ export default class AddCommand {
}

private async handleOAuth(
app: any,
app: Record<string, unknown>,
authMode: string,
authScheme: any,
config: Record<string, any>,
authScheme: TAuthScheme,
config: Record<string, unknown>,
integrationName: string,
noBrowser: boolean,
scopes: string[],
useComposioAuth: boolean
) {
if (useComposioAuth) {
return this.setupIntegration(
app,
app as {
appId: string;
},
authMode,
useComposioAuth,
{},
integrationName
);
}

const authConfig = await this.collectInputFields(authScheme.fields);
const authConfig = await this.collectInputFields(
authScheme.fields as TInputField[]
);

if (scopes) {
authConfig.scopes = scopes.join(",");
}

return this.setupIntegration(
app,
app as {
appId: string;
},
authMode,
useComposioAuth,
authConfig,
Expand All @@ -375,17 +399,10 @@ export default class AddCommand {
}

async collectInputFields(
fields: {
name: string;
displayName: string;
display_name: string;
expected_from_customer: boolean;
required: boolean;
type: string;
}[],
fields: TInputField[],
isConnection = false
): Promise<Record<string, any>> {
const config: Record<string, any> = {};
): Promise<Record<string, unknown>> {
const config: Record<string, unknown> = {};

for (const field of fields) {
if (field.expected_from_customer && !isConnection) {
Expand All @@ -395,7 +412,7 @@ export default class AddCommand {
const { [field.name]: value } = await inquirer.prompt({
type: "input",
name: field.name,
message: field.displayName || field.display_name,
message: (field.displayName || field.display_name) as string,
});

if (value) {
Expand All @@ -407,10 +424,12 @@ export default class AddCommand {
}

async setupIntegration(
app: any,
authMode: any,
app: {
appId: string;
},
authMode: string,
useComposioAuth: boolean,
config: Record<string, any>,
config: Record<string, unknown>,
name: string
) {
const composioClient = new Composio({});
Expand Down
4 changes: 2 additions & 2 deletions js/src/cli/apps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@
import chalk from "chalk";
import { Command } from "commander";

import { getOpenAPIClient } from "../sdk/utils/config";
import client from "../sdk/client/client";
import { getOpenAPIClient } from "../sdk/utils/config";

import resolvePackagePath from "resolve-package-path";
import fs from "fs";
import path from "path";
import resolvePackagePath from "resolve-package-path";

type ErrorWithMessage = {
message: string;
Expand Down
8 changes: 5 additions & 3 deletions js/src/cli/connections.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export default class ConnectionsCommand {
});

if (error) {
console.log(chalk.red((error as any).message));
console.log(chalk.red((error as Error).message));
return;
}

Expand Down Expand Up @@ -66,11 +66,13 @@ export class ConnectionsGetCommand {
});

if (error) {
console.log(chalk.red((error as any).message));
console.log(chalk.red((error as Error).message));
return;
}

for (const [key, value] of Object.entries(data as Record<string, any>)) {
for (const [key, value] of Object.entries(
data as Record<string, unknown>
)) {
console.log(
`- ${chalk.cyan.bold(key)}: ${JSON.stringify(value, null, 2)}`
);
Expand Down
Loading
Loading