From 227421b324bcf5548d1263db7b61fc66eb407908 Mon Sep 17 00:00:00 2001 From: Steve Gill Date: Tue, 17 Dec 2019 18:30:02 -0800 Subject: [PATCH 1/5] improved type resolution for action method --- src/App.ts | 40 ++++++++++++++++++++++------------------ 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/src/App.ts b/src/App.ts index ab2f6aaa7..9d9c0cd2f 100644 --- a/src/App.ts +++ b/src/App.ts @@ -86,11 +86,11 @@ export interface AuthorizeResult { [key: string]: any; } -export interface ActionConstraints { - type?: string; - block_id?: string | RegExp; - action_id?: string | RegExp; - callback_id?: string | RegExp; +export interface ActionConstraints { + type?: A['type']; + block_id?: A extends BlockAction ? (string | RegExp) : never; + action_id?: A extends BlockAction ? (string | RegExp) : never; + callback_id?: Extract extends any ? (string | RegExp) : never; } export interface ViewConstraints { @@ -265,19 +265,23 @@ export default class App { // NOTE: this is what's called a convenience generic, so that types flow more easily without casting. // https://basarat.gitbooks.io/typescript/docs/types/generics.html#design-pattern-convenience-generic - public action( - actionId: string | RegExp, - ...listeners: Middleware>[] - ): void; - public action( - constraints: ActionConstraints, - ...listeners: Middleware>[] - ): void; - public action( - actionIdOrConstraints: string | RegExp | ActionConstraints, - ...listeners: Middleware>[] - ): void { - const constraints: ActionConstraints = + public action( + actionId: string | RegExp, + ...listeners: Middleware>[] + ): void; + public action = ActionConstraints>( + constraints: Constraints, + // NOTE: Extract<> is able to return the whole union when type: undefined. Why? + ...listeners: Middleware>>[] + ): void; + public action = ActionConstraints>( + actionIdOrConstraints: string | RegExp | Constraints, + ...listeners: Middleware>>[] + ): void { + // Normalize Constraints + const constraints: ActionConstraints = (typeof actionIdOrConstraints === 'string' || util.types.isRegExp(actionIdOrConstraints)) ? { action_id: actionIdOrConstraints } : actionIdOrConstraints; From 5ae4ba50f82f164b9674cad53083f504fa3206d7 Mon Sep 17 00:00:00 2001 From: Steve Gill Date: Wed, 18 Dec 2019 12:56:44 -0800 Subject: [PATCH 2/5] added dtslint tests to integration folder --- integration-tests/.gitignore | 3 ++ integration-tests/.vscode/launch.json | 39 ++++++++++++++++ integration-tests/README.md | 22 +++++++++ integration-tests/package.json | 19 ++++++++ integration-tests/types/action.ts | 66 +++++++++++++++++++++++++++ integration-tests/types/index.d.ts | 2 + integration-tests/types/tsconfig.json | 12 +++++ 7 files changed, 163 insertions(+) create mode 100644 integration-tests/.gitignore create mode 100644 integration-tests/.vscode/launch.json create mode 100644 integration-tests/README.md create mode 100644 integration-tests/package.json create mode 100644 integration-tests/types/action.ts create mode 100644 integration-tests/types/index.d.ts create mode 100644 integration-tests/types/tsconfig.json diff --git a/integration-tests/.gitignore b/integration-tests/.gitignore new file mode 100644 index 000000000..d83e85e86 --- /dev/null +++ b/integration-tests/.gitignore @@ -0,0 +1,3 @@ +# node / npm stuff +/node_modules +/package-lock.json diff --git a/integration-tests/.vscode/launch.json b/integration-tests/.vscode/launch.json new file mode 100644 index 000000000..9d1cf678a --- /dev/null +++ b/integration-tests/.vscode/launch.json @@ -0,0 +1,39 @@ +{ + // Use IntelliSense to learn about possible attributes. + // Hover to view descriptions of existing attributes. + // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387 + "version": "0.2.0", + "configurations": [ + { + "type": "node", + "request": "launch", + "name": "Integration Tests", + "program": "${workspaceFolder}/node_modules/mocha/bin/_mocha", + "args": [ + "--timeout", + "999999", + "--colors", + "${workspaceFolder}/proxy-test.js" + ], + "internalConsoleOptions": "openOnSessionStart", + "skipFiles": [ + "/**" + ], + "cwd": "${workspaceFolder}" + }, + { + "type": "node", + "request": "launch", + "name": "Integration Type Checker", + "program": "${workspaceFolder}/node_modules/dtslint/bin/index.js", + "args": [ + "types" + ], + "internalConsoleOptions": "openOnSessionStart", + "skipFiles": [ + "/**" + ], + "cwd": "${workspaceFolder}" + }, + ] +} \ No newline at end of file diff --git a/integration-tests/README.md b/integration-tests/README.md new file mode 100644 index 000000000..fde02efde --- /dev/null +++ b/integration-tests/README.md @@ -0,0 +1,22 @@ +## Integration tests + +This directory contains a minimal project used as an environment in which to run integration tests. + +It's important to isolate this test project from the overall project where the package is built in order to eliminate +assumptions. Specifically, the `package.json` and other configuration files in this directory are meant to include +the minimum requirements and nothing more. Here are a few ways this is set up: + +* The `@types/node` dependency is set to a version that corresponds to the lowest supported node version. This enforces + that the distribution should work in projects who have chosen that particular version, since its the most likely to + not have types that we accidentally depend on but shouldn't. + +* The `typescript` development dependency is set to the most modern version of typescript. It's not important to use the + oldest supported version because the `dtslint` tool will run the typechecker result integration tests on each + supported version of TypeScript. It reads the oldest supported version from `types/index.d.ts`. + +* The `tsconfig.json` file is specifically set to the most strict settings as well as the most minimal requirements. + This is specifically chosen to verify that the requirements expressed in the documentation are sufficient and + represent the least amount of configuration needed to get the package working. + +* In `package.json`, the `"private"` key must remain set to `true`. This enforces that this project doesn't get + accidentally published. diff --git a/integration-tests/package.json b/integration-tests/package.json new file mode 100644 index 000000000..be6154594 --- /dev/null +++ b/integration-tests/package.json @@ -0,0 +1,19 @@ +{ + "name": "@slack/bolt-integration-tests", + "private": true, + "description": "Integration tests for Bolt", + "author": "Slack Technologies, Inc.", + "license": "MIT", + "scripts": { + "test": "npm run test:typechecker", + "test:typechecker": "dtslint types" + }, + "dependencies": { + "@slack/bolt": "^1.4.1", + "@types/node": "^8.10.43" + }, + "devDependencies": { + "dtslint": "^0.5.4", + "typescript": "^3.3.3333" + } +} diff --git a/integration-tests/types/action.ts b/integration-tests/types/action.ts new file mode 100644 index 000000000..96cb5d997 --- /dev/null +++ b/integration-tests/types/action.ts @@ -0,0 +1,66 @@ +// import App, { ActionConstraints } from '../../src/App'; +// import { MessageAction } from '../../src/types'; +import { App } from '@slack/bolt'; + +const app = new App({ token: 'TOKEN', signingSecret: 'Signing Secret' }); + +// calling action method with incorrect an type constraint value should not work +// $ExpectError +app.action({ type: 'Something wrong' }, ({ action }) => { + return action; +}); + +/* +// Should error because message_action doesn't have type action_id +// $ Expect Error +app.action({ type: 'message_action', action_id: 'dafasf' }, ({ action }) => { + // NOT WORKING + return action; +}); +*/ + +// Action in listner should be - MessageAction +// $ExpectType void +app.action({ type: 'message_action' }, ({ + action, // $ExpectType MessageAction + }) => { + return action; +}); + +// $ExpectType void +app.action({ type: 'block_actions' }, ({ + action, // $ExpectType BlockElementAction + }) => { + return action; +}); + +// $ExpectType void +app.action({ type: 'interactive_message' }, ({ + action, // $ExpectType InteractiveAction + }) => { + return action; +}); + +// $ExpectType void +app.action({ type: 'dialog_submission' }, ({ + action, // $ExpectType DialogSubmitAction + }) => { + return action; +}); + +/* TODO: do we import MessageAction from local bolt? +// If action is parameterized with MessageAction, action argument in callback should be type MessageAction +// $ Expect Type void +app.action({}, ({ + action // $ Expect Type MessageAction + }) => { + return action; +}); + +// Should error because MessageAction doesn't have an action_id +// $ Expect Error +app.actiong({ action_id: 'dafasf' }, ({ action }) => { + // NOT WORKING + return action; +}); +*/ diff --git a/integration-tests/types/index.d.ts b/integration-tests/types/index.d.ts new file mode 100644 index 000000000..83e6228d0 --- /dev/null +++ b/integration-tests/types/index.d.ts @@ -0,0 +1,2 @@ +// tslint:disable:no-useless-files +// TypeScript Version: 3.3 diff --git a/integration-tests/types/tsconfig.json b/integration-tests/types/tsconfig.json new file mode 100644 index 000000000..1c0a1abcf --- /dev/null +++ b/integration-tests/types/tsconfig.json @@ -0,0 +1,12 @@ +{ + "compilerOptions": { + "target": "es5", + "module": "commonjs", + "strict": true, + "noUnusedLocals": true, + "noUnusedParameters": true, + "noImplicitReturns": true, + "noFallthroughCasesInSwitch": true, + "lib": ["es5"] + } +} From fee10f9389cbe73826bbd1e6042808c18558a14e Mon Sep 17 00:00:00 2001 From: Steve Gill Date: Wed, 18 Dec 2019 13:05:13 -0800 Subject: [PATCH 3/5] fixed up launch.json --- integration-tests/.vscode/launch.json | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/integration-tests/.vscode/launch.json b/integration-tests/.vscode/launch.json index 9d1cf678a..5abdf0670 100644 --- a/integration-tests/.vscode/launch.json +++ b/integration-tests/.vscode/launch.json @@ -4,23 +4,6 @@ // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387 "version": "0.2.0", "configurations": [ - { - "type": "node", - "request": "launch", - "name": "Integration Tests", - "program": "${workspaceFolder}/node_modules/mocha/bin/_mocha", - "args": [ - "--timeout", - "999999", - "--colors", - "${workspaceFolder}/proxy-test.js" - ], - "internalConsoleOptions": "openOnSessionStart", - "skipFiles": [ - "/**" - ], - "cwd": "${workspaceFolder}" - }, { "type": "node", "request": "launch", From a35d315294bce3fb8fb3ee8dbd772291fd50b193 Mon Sep 17 00:00:00 2001 From: Steve Gill Date: Wed, 18 Dec 2019 18:15:04 -0800 Subject: [PATCH 4/5] updated PR based on feedback --- integration-tests/.vscode/launch.json | 33 ++++++++++++++++++++++++++- integration-tests/package.json | 6 ++--- integration-tests/types/action.ts | 12 ++++------ 3 files changed, 40 insertions(+), 11 deletions(-) diff --git a/integration-tests/.vscode/launch.json b/integration-tests/.vscode/launch.json index 5abdf0670..4ded08d73 100644 --- a/integration-tests/.vscode/launch.json +++ b/integration-tests/.vscode/launch.json @@ -18,5 +18,36 @@ ], "cwd": "${workspaceFolder}" }, + { + "type": "node", + "request": "launch", + "name": "npm test", + "cwd": "${workspaceFolder}", + "runtimeExecutable": "npm", + "runtimeArgs": [ + "run-script", "test" + ], + "port": 9229 + + }, + ] +} + +/* +[ + { + "type": "node", + "request": "launch", + "name": "Integration Type Checker", + "program": "${workspaceFolder}/node_modules/dtslint/bin/index.js", + "args": [ + "types" + ], + "internalConsoleOptions": "openOnSessionStart", + "skipFiles": [ + "/**" + ], + "cwd": "${workspaceFolder}" + }, ] -} \ No newline at end of file +*/ diff --git a/integration-tests/package.json b/integration-tests/package.json index be6154594..29fe84b66 100644 --- a/integration-tests/package.json +++ b/integration-tests/package.json @@ -9,11 +9,11 @@ "test:typechecker": "dtslint types" }, "dependencies": { - "@slack/bolt": "^1.4.1", - "@types/node": "^8.10.43" + "@slack/bolt": "file:..", + "@types/node": "^10.0.0" }, "devDependencies": { "dtslint": "^0.5.4", - "typescript": "^3.3.3333" + "typescript": "^3.7.3" } } diff --git a/integration-tests/types/action.ts b/integration-tests/types/action.ts index 96cb5d997..9f70c83cc 100644 --- a/integration-tests/types/action.ts +++ b/integration-tests/types/action.ts @@ -1,6 +1,5 @@ // import App, { ActionConstraints } from '../../src/App'; -// import { MessageAction } from '../../src/types'; -import { App } from '@slack/bolt'; +import { App, MessageAction } from '@slack/bolt'; const app = new App({ token: 'TOKEN', signingSecret: 'Signing Secret' }); @@ -10,11 +9,10 @@ app.action({ type: 'Something wrong' }, ({ action }) => { return action; }); -/* +/* Not working // Should error because message_action doesn't have type action_id // $ Expect Error app.action({ type: 'message_action', action_id: 'dafasf' }, ({ action }) => { - // NOT WORKING return action; }); */ @@ -48,15 +46,15 @@ app.action({ type: 'dialog_submission' }, ({ return action; }); -/* TODO: do we import MessageAction from local bolt? // If action is parameterized with MessageAction, action argument in callback should be type MessageAction -// $ Expect Type void +// $ExpectType void app.action({}, ({ - action // $ Expect Type MessageAction + action // $ExpectType MessageAction }) => { return action; }); +/* Not working // Should error because MessageAction doesn't have an action_id // $ Expect Error app.actiong({ action_id: 'dafasf' }, ({ action }) => { From 8bb683809bb329cc7c644c1bc898f36c9a6a4235 Mon Sep 17 00:00:00 2001 From: Steve Gill Date: Thu, 19 Dec 2019 11:25:41 -0800 Subject: [PATCH 5/5] added integration tests to top level npm test command --- package.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index f728ba60d..c366f6d8a 100644 --- a/package.json +++ b/package.json @@ -30,7 +30,8 @@ "build:clean": "shx rm -rf ./dist ./coverage ./.nyc_output", "lint": "tslint --project .", "mocha": "nyc mocha --config .mocharc.json \"src/**/*.spec.ts\"", - "test": "npm run lint && npm run mocha", + "test": "npm run lint && npm run mocha && npm run test:integration", + "test:integration": "cd integration-tests && npm install && npm test", "coverage": "codecov" }, "repository": "slackapi/bolt",