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

Update code to make it TS 4.4 compatible #1319

Merged
merged 1 commit into from
Aug 27, 2021
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
4 changes: 2 additions & 2 deletions packages/oauth/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
},
"dependencies": {
"@slack/logger": "^3.0.0",
"@slack/web-api": "^6.0.0",
"@slack/web-api": "^6.3.0",
Copy link
Member Author

Choose a reason for hiding this comment

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

This has not been safe enough. oauth.v2.access API argments in the code are compatible only with 6.3+

"@types/jsonwebtoken": "^8.3.7",
"@types/node": ">=12",
"jsonwebtoken": "^8.5.1",
Expand All @@ -64,4 +64,4 @@
"typescript": "^4.1",
"uncaughtException": "^1.0.0"
}
}
}
4 changes: 2 additions & 2 deletions packages/oauth/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ export class InstallProvider {
}

return authResult;
} catch (error) {
} catch (error: any) {
Copy link
Member Author

Choose a reason for hiding this comment

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

As either any or unknown is allowed for the type, we are unable to use more specific types here. If we want to do so, we have to cast the type from unknown inside the catch clause.

throw new AuthorizationError(error.message);
}
}
Expand Down Expand Up @@ -453,7 +453,7 @@ export class InstallProvider {
this.logger.debug('run built-in success function');
callbackSuccess(installation, installOptions, req, res);
}
} catch (error) {
} catch (error: any) {
this.logger.error(error);

// Call the failure callback
Expand Down
2 changes: 1 addition & 1 deletion packages/rtm-api/src/KeepAlive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ export class KeepAlive extends EventEmitter {
.catch((error) => {
this.logger.error(`Unhandled error: ${error.message}. Please report to @slack/rtm-api package maintainers.`);
});
} catch (error) {
} catch (error: any) {
this.logger.error(`Unhandled error: ${error.message}. Please report to @slack/rtm-api package maintainers.`);
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/rtm-api/src/RTMClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -596,7 +596,7 @@ export class RTMClient extends EventEmitter {
let event;
try {
event = JSON.parse(data);
} catch (parseError) {
} catch (parseError: any) {
// prevent application from crashing on a bad message, but log an error to bring attention
this.logger.error(
`unable to parse incoming websocket message: ${parseError.message}\n message contents: "${data}"`,
Expand Down
2 changes: 1 addition & 1 deletion packages/socket-mode/src/SocketModeClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -542,7 +542,7 @@ export class SocketModeClient extends EventEmitter {

try {
event = JSON.parse(data);
} catch (parseError) {
} catch (parseError: any) {
// prevent application from crashing on a bad message, but log an error to bring attention
this.logger.error(
`unable to parse incoming websocket message: ${parseError.message}`,
Expand Down
10 changes: 6 additions & 4 deletions packages/web-api/src/WebClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -359,9 +359,11 @@ export class WebClient extends Methods {

return response;
} catch (error) {
this.logger.warn('http request failed', error.message);
if (error.request) {
throw requestErrorWithOriginal(error);
// To make this compatible with tsd, casting here instead of `catch (error: any)`
Copy link
Member Author

Choose a reason for hiding this comment

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

We run tsd type tests only for web-api package as of today

const e = error as any;
this.logger.warn('http request failed', e.message);
if (e.request) {
throw requestErrorWithOriginal(e);
}
throw error;
}
Expand Down Expand Up @@ -571,7 +573,7 @@ function paginationOptionsForNextPage(
cursor: previousResult.response_metadata.next_cursor as string,
};
}
return;
return undefined;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@ expectError(web.paginate('conversations.list', {}, () => 7));

expectType<Promise<number>>(web.paginate('conversations.list', {}, () => false, () => 5));

// NOTE: this error does not arise with TypeScript 4.4+
// When there's no shouldStop predicate given but there is a reducer, the behavior is undefined.
// (However in the current implementation, the return value is `AsyncIteratable<WebAPICallResult>`.)
expectError(web.paginate('conversations.list', {}, undefined, () => 5));
// expectError(web.paginate('conversations.list', {}, undefined, () => 5));

// Ensure that it works in a for-await-of loop.
async () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/webhook/src/IncomingWebhook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export class IncomingWebhook {
try {
const response = await this.axios.post(this.url, payload);
return this.buildResult(response);
} catch (error) {
} catch (error: any) {
// Wrap errors in this packages own error types (abstract the implementation details' types)
if (error.response !== undefined) {
throw httpErrorWithOriginal(error);
Expand Down