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

improved type resolution for action method #349

Merged
merged 5 commits into from
Dec 19, 2019
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
3 changes: 3 additions & 0 deletions integration-tests/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# node / npm stuff
/node_modules
/package-lock.json
53 changes: 53 additions & 0 deletions integration-tests/.vscode/launch.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
{
// 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 Type Checker",
"program": "${workspaceFolder}/node_modules/dtslint/bin/index.js",
Copy link
Contributor

Choose a reason for hiding this comment

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

should this be the npm test script instead of directly trying to run dtslint?

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried that by following https://code.visualstudio.com/docs/nodejs/nodejs-debugging, but it seems very specific to the debug command. I've tried swapping it for npm test, but it just hangs.

        {
            "type": "node",
            "request": "launch",
            "name": "npm test",
            "cwd": "${workspaceFolder}",
            "runtimeExecutable": "npm",
            "runtimeArgs": [
                "run-script", "test"
            ],
            "port": 9229
        },

"args": [
"types"
],
"internalConsoleOptions": "openOnSessionStart",
"skipFiles": [
"<node_internals>/**"
],
"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": [
"<node_internals>/**"
],
"cwd": "${workspaceFolder}"
},
]
*/
22 changes: 22 additions & 0 deletions integration-tests/README.md
Original file line number Diff line number Diff line change
@@ -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.
19 changes: 19 additions & 0 deletions integration-tests/package.json
Original file line number Diff line number Diff line change
@@ -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": "file:..",
"@types/node": "^10.0.0"
},
"devDependencies": {
"dtslint": "^0.5.4",
"typescript": "^3.7.3"
}
}
64 changes: 64 additions & 0 deletions integration-tests/types/action.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
// import App, { ActionConstraints } from '../../src/App';
import { App, MessageAction } 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;
});

/* Not working
// Should error because message_action doesn't have type action_id
// $ Expect Error
app.action({ type: 'message_action', action_id: 'dafasf' }, ({ action }) => {
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
aoberoi marked this conversation as resolved.
Show resolved Hide resolved
}) => {
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;
});

// If action is parameterized with MessageAction, action argument in callback should be type MessageAction
// $ExpectType void
app.action<MessageAction>({}, ({
action // $ExpectType MessageAction
}) => {
return action;
});

/* Not working
// Should error because MessageAction doesn't have an action_id
// $ Expect Error
app.actiong<MessageAction>({ action_id: 'dafasf' }, ({ action }) => {
// NOT WORKING
return action;
});
*/
2 changes: 2 additions & 0 deletions integration-tests/types/index.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// tslint:disable:no-useless-files
// TypeScript Version: 3.3
12 changes: 12 additions & 0 deletions integration-tests/types/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"compilerOptions": {
"target": "es5",
"module": "commonjs",
"strict": true,
"noUnusedLocals": true,
"noUnusedParameters": true,
"noImplicitReturns": true,
"noFallthroughCasesInSwitch": true,
"lib": ["es5"]
}
}
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
40 changes: 22 additions & 18 deletions src/App.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<A extends SlackAction = SlackAction> {
type?: A['type'];
block_id?: A extends BlockAction ? (string | RegExp) : never;
action_id?: A extends BlockAction ? (string | RegExp) : never;
callback_id?: Extract<A, { callback_id?: string }> extends any ? (string | RegExp) : never;
}

export interface ViewConstraints {
Expand Down Expand Up @@ -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<ActionType extends SlackAction = SlackAction>(
actionId: string | RegExp,
...listeners: Middleware<SlackActionMiddlewareArgs<ActionType>>[]
): void;
public action<ActionType extends SlackAction = SlackAction>(
constraints: ActionConstraints,
...listeners: Middleware<SlackActionMiddlewareArgs<ActionType>>[]
): void;
public action<ActionType extends SlackAction = SlackAction>(
actionIdOrConstraints: string | RegExp | ActionConstraints,
...listeners: Middleware<SlackActionMiddlewareArgs<ActionType>>[]
): void {
const constraints: ActionConstraints =
public action<Action extends SlackAction = SlackAction>(
actionId: string | RegExp,
...listeners: Middleware<SlackActionMiddlewareArgs<Action>>[]
): void;
public action<Action extends SlackAction = SlackAction,
Constraints extends ActionConstraints<Action> = ActionConstraints<Action>>(
constraints: Constraints,
// NOTE: Extract<> is able to return the whole union when type: undefined. Why?
...listeners: Middleware<SlackActionMiddlewareArgs<Extract<Action, { type: Constraints['type'] }>>>[]
): void;
public action<Action extends SlackAction = SlackAction,
Constraints extends ActionConstraints<Action> = ActionConstraints<Action>>(
actionIdOrConstraints: string | RegExp | Constraints,
...listeners: Middleware<SlackActionMiddlewareArgs<Extract<Action, { type: Constraints['type'] }>>>[]
): void {
// Normalize Constraints
const constraints: ActionConstraints =
(typeof actionIdOrConstraints === 'string' || util.types.isRegExp(actionIdOrConstraints)) ?
{ action_id: actionIdOrConstraints } : actionIdOrConstraints;

Expand Down