Skip to content

Commit

Permalink
Fix slackapi#1098 next() is optional in middleware in TypeScript
Browse files Browse the repository at this point in the history
  • Loading branch information
seratch committed Aug 30, 2021
1 parent 992b64a commit 41bb74c
Show file tree
Hide file tree
Showing 7 changed files with 30 additions and 63 deletions.
18 changes: 9 additions & 9 deletions src/App.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -455,8 +455,8 @@ describe('App', () => {
// Arrange
app.use(fakeFirstMiddleware);
app.use(async ({ next }) => {
await next!();
await next!();
await next();
await next();
});
app.use(fakeSecondMiddleware);
app.error(fakeErrorHandler);
Expand All @@ -475,7 +475,7 @@ describe('App', () => {
await delay(100);
changed = true;

await next!();
await next();
});

await fakeReceiver.sendEvent(dummyReceiverEvent);
Expand All @@ -489,7 +489,7 @@ describe('App', () => {

app.use(async ({ next }) => {
try {
await next!();
await next();
} catch (err: any) {
caughtError = err;
}
Expand Down Expand Up @@ -1188,7 +1188,7 @@ describe('App', () => {

app.use(async ({ next }) => {
await ackFn();
await next!();
await next();
});
app.shortcut({ callback_id: 'message_action_callback_id' }, async () => {
await shortcutFn();
Expand Down Expand Up @@ -1289,7 +1289,7 @@ describe('App', () => {

app.use(async ({ next }) => {
await ackFn();
await next!();
await next();
});
app.shortcut({ callback_id: 'message_action_callback_id' }, async () => {
await shortcutFn();
Expand Down Expand Up @@ -1607,7 +1607,7 @@ describe('App', () => {
});
app.use(async ({ logger, body, next }) => {
logger.info(body);
await next!();
await next();
});

app.event('app_home_opened', async ({ logger, event }) => {
Expand Down Expand Up @@ -1655,7 +1655,7 @@ describe('App', () => {
});
app.use(async ({ logger, body, next }) => {
logger.info(body);
await next!();
await next();
});

app.event('app_home_opened', async ({ logger, event }) => {
Expand Down Expand Up @@ -1718,7 +1718,7 @@ describe('App', () => {
});
app.use(async ({ client, next }) => {
await client.auth.test();
await next!();
await next();
});
const clients: WebClient[] = [];
app.event('app_home_opened', async ({ client }) => {
Expand Down
4 changes: 3 additions & 1 deletion src/App.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ import {
} from './types';
import { IncomingEventType, getTypeAndConversation, assertNever } from './helpers';
import { CodedError, asCodedError, AppInitializationError, MultipleListenerError, ErrorCode } from './errors';
import { AllMiddlewareArgs } from './types/middleware';
// eslint-disable-next-line import/order
import allSettled = require('promise.allsettled'); // eslint-disable-line @typescript-eslint/no-require-imports
// eslint-disable-next-line @typescript-eslint/no-require-imports, import/no-commonjs
Expand Down Expand Up @@ -886,7 +887,8 @@ export default class App {
context,
client,
logger: this.logger,
}),
// `next` is already set in the outer processMiddleware
} as AnyMiddlewareArgs & AllMiddlewareArgs),
);
});

Expand Down
8 changes: 2 additions & 6 deletions src/WorkflowStep.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -291,12 +291,12 @@ describe('WorkflowStep', () => {

describe('processStepMiddleware', () => {
it('should call each callback in user-provided middleware', async () => {
const { next: _next, ...fakeArgs } = createFakeStepEditAction() as unknown as AllWorkflowStepMiddlewareArgs;
const { ...fakeArgs } = createFakeStepEditAction() as unknown as AllWorkflowStepMiddlewareArgs;
const { processStepMiddleware } = await importWorkflowStep();

const fn1 = sinon.spy((async ({ next: continuation }) => {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
await continuation!();
await continuation();
}) as Middleware<WorkflowStepEdit>);
const fn2 = sinon.spy(async () => {});
const fakeMiddleware = [fn1, fn2] as WorkflowStepMiddleware;
Expand All @@ -323,7 +323,6 @@ function createFakeStepEditAction() {
workflow_step: {},
},
context: {},
next: sinon.fake(),
};
}

Expand All @@ -341,7 +340,6 @@ function createFakeStepSaveEvent() {
callback_id: 'test_save_callback_id',
},
context: {},
next: sinon.fake(),
};
}

Expand All @@ -362,7 +360,6 @@ function createFakeStepExecuteEvent() {
},
},
context: {},
next: sinon.fake(),
};
}

Expand All @@ -380,6 +377,5 @@ function createFakeViewEvent() {
callback_id: 'test_view_callback_id',
},
context: {},
next: sinon.fake(),
};
}
4 changes: 1 addition & 3 deletions src/WorkflowStep.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,9 +154,7 @@ export class WorkflowStep {
if (isStepEvent(args) && this.matchesConstraints(args)) {
return this.processEvent(args);
}
// `next` here exists for sure bu the typing on the MiddlwareArgs is not great enough
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
return args.next!();
return args.next();
};
}

Expand Down
4 changes: 1 addition & 3 deletions src/conversation-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,6 @@ export function conversationContext<ConversationState = any>(
} else {
logger.debug('No conversation ID for incoming event');
}
// TODO: remove the non-null assertion operator
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
await next!();
await next();
};
}
52 changes: 13 additions & 39 deletions src/middleware/builtin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,7 @@ export const onlyActions: Middleware<AnyMiddlewareArgs & { action?: SlackAction
}

// It matches so we should continue down this middleware listener chain
// TODO: remove the non-null assertion operator
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
await next!();
await next();
};

/**
Expand All @@ -57,9 +55,7 @@ export const onlyShortcuts: Middleware<AnyMiddlewareArgs & { shortcut?: SlackSho
}

// It matches so we should continue down this middleware listener chain
// TODO: remove the non-null assertion operator
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
await next!();
await next();
};

/**
Expand All @@ -72,9 +68,7 @@ export const onlyCommands: Middleware<AnyMiddlewareArgs & { command?: SlashComma
}

// It matches so we should continue down this middleware listener chain
// TODO: remove the non-null assertion operator
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
await next!();
await next();
};

/**
Expand All @@ -87,9 +81,7 @@ export const onlyOptions: Middleware<AnyMiddlewareArgs & { options?: SlackOption
}

// It matches so we should continue down this middleware listener chain
// TODO: remove the non-null assertion operator
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
await next!();
await next();
};

/**
Expand All @@ -102,9 +94,7 @@ export const onlyEvents: Middleware<AnyMiddlewareArgs & { event?: SlackEvent }>
}

// It matches so we should continue down this middleware listener chain
// TODO: remove the non-null assertion operator
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
await next!();
await next();
};

/**
Expand All @@ -117,9 +107,7 @@ export const onlyViewActions: Middleware<AnyMiddlewareArgs & { view?: ViewOutput
}

// It matches so we should continue down this middleware listener chain
// TODO: remove the non-null assertion operator
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
await next!();
await next();
};

/**
Expand Down Expand Up @@ -206,9 +194,7 @@ export function matchConstraints(
if (body.type !== constraints.type) return;
}

// TODO: remove the non-null assertion operator
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
await next!();
await next();
};
}

Expand Down Expand Up @@ -240,9 +226,7 @@ export function matchMessage(
}
}

// TODO: remove the non-null assertion operator
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
await next!();
await next();
};
}

Expand All @@ -256,9 +240,7 @@ export function matchCommandName(pattern: string | RegExp): Middleware<SlackComm
return;
}

// TODO: remove the non-null assertion operator
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
await next!();
await next();
};
}

Expand Down Expand Up @@ -294,9 +276,7 @@ export function matchEventType(pattern: EventTypePattern): Middleware<SlackEvent
}
}

// TODO: remove the non-null assertion operator
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
await next!();
await next();
};
}

Expand Down Expand Up @@ -329,18 +309,14 @@ export function ignoreSelf(): Middleware<AnyMiddlewareArgs> {
}

// If all the previous checks didn't skip this message, then its okay to resume to next
// TODO: remove the non-null assertion operator
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
await args.next!();
await args.next();
};
}

export function subtype(subtype1: string): Middleware<SlackEventMiddlewareArgs<'message'>> {
return async ({ message, next }) => {
if (message.subtype === subtype1) {
// TODO: remove the non-null assertion operator
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
await next!();
await next();
}
};
}
Expand Down Expand Up @@ -376,9 +352,7 @@ export function directMention(): Middleware<SlackEventMiddlewareArgs<'message'>>
return;
}

// TODO: remove the non-null assertion operator
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
await next!();
await next();
};
}

Expand Down
3 changes: 1 addition & 2 deletions src/types/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,7 @@ export interface AllMiddlewareArgs {
context: Context;
logger: Logger;
client: WebClient;
// TODO: figure out how to make next non-optional
next?: NextFn;
next: NextFn;
}

// NOTE: Args should extend AnyMiddlewareArgs, but because of contravariance for function types, including that as a
Expand Down

0 comments on commit 41bb74c

Please sign in to comment.