Skip to content

Commit

Permalink
@slack/web-api argument type safety (#1673)
Browse files Browse the repository at this point in the history
Co-authored-by: Kazuhiro Sera <[email protected]>
  • Loading branch information
filmaj and seratch authored Jan 15, 2024
1 parent 27b78e5 commit d53ef02
Show file tree
Hide file tree
Showing 394 changed files with 8,156 additions and 2,319 deletions.
2 changes: 1 addition & 1 deletion lint-configs/.eslintignore
Original file line number Diff line number Diff line change
@@ -1 +1 @@
node_modules
node_modules
20 changes: 18 additions & 2 deletions lint-configs/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,10 @@ module.exports = {
format: ['camelCase'],
leadingUnderscore: 'allow',
},
{
selector: 'import',
format: null, // do not force conventions on imports
},
{
selector: 'variable',
// PascalCase for variables is added to allow exporting a singleton, function library, or bare object as in
Expand Down Expand Up @@ -246,8 +250,7 @@ module.exports = {
allow: [
'**/middleware/*', // the src/middleware directory doesn't export a module, it's just a namespace.
'**/receivers/*', // the src/receivers directory doesn't export a module, it's just a namespace.
'**/types/**/*',
'**/types/*', // type heirarchies should be used however one wants
'**/types/**/*', // type heirarchies should be used however one wants
],
}],

Expand Down Expand Up @@ -296,6 +299,19 @@ module.exports = {
'symbol-description': 'off',
},
},
{
files: ['test/types/**/*.test-d.ts'],
extends: ['plugin:@typescript-eslint/disable-type-checked'],
rules: {
'import/no-internal-modules': ['error', {
// Use the following option to set a list of allowable globs in this project.
allow: [
'**/src/**/*', // allow type tests to reach into src/
'**/types/**/*', // type heirarchies should be used however one wants
],
}],
}
},
],
};

Expand Down
34 changes: 16 additions & 18 deletions packages/oauth/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
"lint": "eslint --fix --ext .ts src",
"test": "npm run lint && npm run test:mocha",
"test:mocha": "nyc mocha --config .mocharc.json src/*.spec.js src/**/*.spec.js src/*.spec.ts src/**/*.spec.ts",
"coverage": "codecov -F oauthhelper --root=$PWD",
"ref-docs:model": "api-extractor run",
"watch": "npx nodemon --watch 'src' --ext 'ts' --exec npm run build"
},
Expand All @@ -47,29 +46,28 @@
"lodash.isstring": "^4.0.1"
},
"devDependencies": {
"@microsoft/api-extractor": "^7.19.4",
"@types/chai": "^4.2.11",
"@types/mocha": "^9.1.0",
"@types/sinon": "^10.0.11",
"@typescript-eslint/eslint-plugin": "^4.4.1",
"@typescript-eslint/parser": "^4.4.0",
"chai": "^4.2.0",
"codecov": "^3.0.4",
"eslint": "^7.32.0",
"eslint-config-airbnb-base": "^14.2.1",
"eslint-config-airbnb-typescript": "^12.3.1",
"eslint-plugin-import": "^2.22.1",
"eslint-plugin-jsdoc": "^30.6.1",
"@microsoft/api-extractor": "^7.38.0",
"@types/chai": "^4.3.5",
"@types/mocha": "^10.0.1",
"@types/sinon": "^10.0.20",
"@typescript-eslint/eslint-plugin": "^6.4.1",
"@typescript-eslint/parser": "^6.4.0",
"chai": "^4.3.8",
"eslint": "^8.47.0",
"eslint-config-airbnb-base": "^15.0.0",
"eslint-config-airbnb-typescript": "^17.1.0",
"eslint-plugin-import": "^2.28.1",
"eslint-plugin-jsdoc": "^46.5.0",
"eslint-plugin-node": "^11.1.0",
"mocha": "^9.2.1",
"mocha": "^10.2.0",
"nop": "^1.0.0",
"nyc": "^15.1.0",
"rewiremock": "^3.13.9",
"shx": "^0.3.2",
"sinon": "^9.0.2",
"source-map-support": "^0.5.12",
"sinon": "^15.2.0",
"source-map-support": "^0.5.21",
"superagent": "^3.3.1",
"ts-node": "^8.2.0",
"ts-node": "^10.8.1",
"typescript": "^4.1",
"uncaughtException": "^1.0.0"
}
Expand Down
2 changes: 2 additions & 0 deletions packages/oauth/src/callback-options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ export function defaultCallbackSuccess(
<p>Redirecting to the Slack App... click <a href="${escapeHtml(redirectUrl)}">here</a>. If you use the browser version of Slack, click <a href="${escapeHtml(browserUrl)}" target="_blank">this link</a> instead.</p>
</body>
</html>`;
// eslint-disable-next-line @typescript-eslint/naming-convention
res.writeHead(200, { 'Content-Type': 'text/html; charset=utf-8' });
res.end(htmlResponse);
}
Expand All @@ -139,6 +140,7 @@ export function defaultCallbackFailure(
default:
httpStatus = 500;
}
// eslint-disable-next-line @typescript-eslint/naming-convention
res.writeHead(httpStatus, { 'Content-Type': 'text/html; charset=utf-8' });
const html = `<html>
<head>
Expand Down
4 changes: 1 addition & 3 deletions packages/oauth/src/install-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -677,9 +677,7 @@ export class InstallProvider {
// End: Build the installation object

if (options?.afterInstallation !== undefined) {
shouldProceed = await options.afterInstallation(
installation, installOptions, req, res,
);
shouldProceed = await options.afterInstallation(installation, installOptions, req, res);
}
if (!shouldProceed) {
// When options.beforeInstallation returns false,
Expand Down
16 changes: 8 additions & 8 deletions packages/rtm-api/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -54,14 +54,14 @@
"ws": "^7.5.3"
},
"devDependencies": {
"@microsoft/api-extractor": "^7.3.4",
"@typescript-eslint/eslint-plugin": "^4.4.1",
"@typescript-eslint/parser": "^4.4.0",
"eslint": "^7.32.0",
"eslint-config-airbnb-base": "^14.2.1",
"eslint-config-airbnb-typescript": "^12.3.1",
"eslint-plugin-import": "^2.22.1",
"eslint-plugin-jsdoc": "^30.6.1",
"@microsoft/api-extractor": "^7.38.0",
"@typescript-eslint/eslint-plugin": "^6.4.1",
"@typescript-eslint/parser": "^6.4.0",
"eslint": "^8.47.0",
"eslint-config-airbnb-base": "^15.0.0",
"eslint-config-airbnb-typescript": "^17.1.0",
"eslint-plugin-import": "^2.28.1",
"eslint-plugin-jsdoc": "^46.5.0",
"eslint-plugin-node": "^11.1.0",
"shx": "^0.3.2",
"typescript": "^4.1.0"
Expand Down
1 change: 1 addition & 0 deletions packages/rtm-api/src/KeepAlive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@ export class KeepAlive extends EventEmitter {
.catch((error) => {
this.logger.error(`Unhandled error: ${error.message}. Please report to @slack/rtm-api package maintainers.`);
});
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} 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 @@ -484,7 +484,6 @@ export class RTMClient extends EventEmitter {
*
* If the awaitReply parameter is set to false, then the returned Promise is resolved as soon as the message is sent
* from the websocket.
*
* @param awaitReply - whether to wait for an acknowledgement response from the platform before resolving the returned
* Promise.
* @param type - the message type
Expand Down Expand Up @@ -629,6 +628,7 @@ export class RTMClient extends EventEmitter {
let event;
try {
event = JSON.parse(data);
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} catch (parseError: any) {
// prevent application from crashing on a bad message, but log an error to bring attention
this.logger.error(
Expand Down
30 changes: 15 additions & 15 deletions packages/socket-mode/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -56,23 +56,23 @@
"ws": "^7.5.3"
},
"devDependencies": {
"@typescript-eslint/eslint-plugin": "^4.4.1",
"@typescript-eslint/parser": "^4.4.0",
"@types/chai": "^4.1.7",
"@types/mocha": "^5.2.6",
"chai": "^4.2.0",
"eslint": "^7.32.0",
"eslint-config-airbnb-base": "^14.2.1",
"eslint-config-airbnb-typescript": "^12.3.1",
"eslint-plugin-import": "^2.22.1",
"eslint-plugin-jsdoc": "^30.6.1",
"@typescript-eslint/eslint-plugin": "^6.4.1",
"@typescript-eslint/parser": "^6.4.0",
"@types/chai": "^4.3.5",
"@types/mocha": "^10.0.1",
"chai": "^4.3.8",
"eslint": "^8.47.0",
"eslint-config-airbnb-base": "^15.0.0",
"eslint-config-airbnb-typescript": "^17.1.0",
"eslint-plugin-import": "^2.28.1",
"eslint-plugin-jsdoc": "^46.5.0",
"eslint-plugin-node": "^11.1.0",
"mocha": "^9.1.0",
"nyc": "^14.1.1",
"mocha": "^10.2.0",
"nyc": "^15.1.0",
"shx": "^0.3.2",
"ts-node": "^8.2.0",
"sinon": "^7.3.2",
"source-map-support": "^0.5.12",
"sinon": "^15.2.0",
"source-map-support": "^0.5.21",
"ts-node": "^10.8.1",
"typescript": "^4.1.0"
}
}
4 changes: 2 additions & 2 deletions packages/socket-mode/src/SocketModeClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,8 +189,8 @@ export class SocketModeClient extends EventEmitter {
private numOfConsecutiveReconnectionFailures: number = 0;

/* eslint-disable @typescript-eslint/indent, newline-per-chained-call */
private connectingStateMachineConfig: Configuration<ConnectingState, Event>
= Finity.configure<ConnectingState, Event>()
private connectingStateMachineConfig: Configuration<ConnectingState, Event> = Finity
.configure<ConnectingState, Event>()
.global()
.onStateEnter((state) => {
this.logger.debug(`Transitioning to state: ${State.Connecting}:${state}`);
Expand Down
16 changes: 8 additions & 8 deletions packages/types/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,14 @@
"ref-docs:model": "api-extractor run"
},
"devDependencies": {
"@microsoft/api-extractor": "^7.3.4",
"@typescript-eslint/eslint-plugin": "^4.4.1",
"@typescript-eslint/parser": "^4.4.0",
"eslint": "^7.32.0",
"eslint-config-airbnb-base": "^14.2.1",
"eslint-config-airbnb-typescript": "^12.3.1",
"eslint-plugin-import": "^2.22.1",
"eslint-plugin-jsdoc": "^30.6.1",
"@microsoft/api-extractor": "^7.38.0",
"@typescript-eslint/eslint-plugin": "^6.4.1",
"@typescript-eslint/parser": "^6.4.0",
"eslint": "^8.47.0",
"eslint-config-airbnb-base": "^15.0.0",
"eslint-config-airbnb-typescript": "^17.1.0",
"eslint-plugin-import": "^2.28.1",
"eslint-plugin-jsdoc": "^46.5.0",
"eslint-plugin-node": "^11.1.0",
"shx": "^0.3.2",
"typescript": "^4.1.0"
Expand Down
3 changes: 2 additions & 1 deletion packages/web-api/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@slack/web-api",
"version": "6.9.0",
"version": "7.0.0-rc.0",
"description": "Official library for using the Slack Platform's Web API",
"author": "Slack Technologies, LLC",
"license": "MIT",
Expand Down Expand Up @@ -62,6 +62,7 @@
"@microsoft/api-extractor": "^7.38.0",
"@types/chai": "^4.3.5",
"@types/mocha": "^10.0.1",
"@types/sinon": "^10.0.20",
"@typescript-eslint/eslint-plugin": "^6.4.1",
"@typescript-eslint/parser": "^6.4.0",
"busboy": "^1.6.0",
Expand Down
20 changes: 0 additions & 20 deletions packages/web-api/src/WebClient.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -797,26 +797,6 @@ describe('WebClient', function () {
this.method = 'conversations.list';
});

describe('logging', function () {
beforeEach(function () {
this.capture = new CaptureConsole();
this.capture.startCapture();
});
it('should log a warning when called with a method not known to be cursor pagination enabled', function () {
this.client.paginate('method');
const output = this.capture.getCapturedText();
assert.isNotEmpty(output);
});
it('should not log a warning when called with a known cursor pagination enabled', function () {
this.client.paginate(this.method);
const output = this.capture.getCapturedText();
assert.isEmpty(output);
});
afterEach(function () {
this.capture.stopCapture();
});
});

describe('when not given shouldStop predicate', function () {
it('should return an AsyncIterator', function () {
const iterator = this.client.paginate(this.method);
Expand Down
Loading

0 comments on commit d53ef02

Please sign in to comment.