Skip to content

Commit

Permalink
Fix #1098 next() is optional in middleware in TypeScript
Browse files Browse the repository at this point in the history
  • Loading branch information
@seratch
seratch committed Oct 6, 2021
1 parent a24e3b8 commit e8933b7
Show file tree
Hide file tree
Showing 7 changed files with 32 additions and 65 deletions.
18 changes: 9 additions & 9 deletions src/App.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -483,8 +483,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 @@ -503,7 +503,7 @@ describe('App', () => {
await delay(100);
changed = true;

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

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

app.use(async ({ next }) => {
try {
await next!();
await next();
} catch (err: any) {
caughtError = err;
}
Expand Down Expand Up @@ -1216,7 +1216,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 @@ -1317,7 +1317,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 @@ -1635,7 +1635,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 @@ -1683,7 +1683,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 @@ -1746,7 +1746,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
7 changes: 5 additions & 2 deletions src/App.ts
View file
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 @@ -891,13 +892,15 @@ export default class App {
context,
client,
this.logger,
// When the listener middleware chain is done processing, call the listener without a next fn
// When all of the listener middleware are done processing,
// `listener` here will be called without a `next` execution
async () => listener({
...(listenerArgs as AnyMiddlewareArgs),
context,
client,
logger: this.logger,
}),
// `next` is already set in the outer processMiddleware
} as AnyMiddlewareArgs & AllMiddlewareArgs),
);
});

Expand Down
9 changes: 2 additions & 7 deletions src/WorkflowStep.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -291,12 +291,11 @@ 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 +322,6 @@ function createFakeStepEditAction() {
workflow_step: {},
},
context: {},
next: sinon.fake(),
};
}

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

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

Expand All @@ -380,6 +376,5 @@ function createFakeViewEvent() {
callback_id: 'test_view_callback_id',
},
context: {},
next: sinon.fake(),
};
}
4 changes: 1 addition & 3 deletions src/WorkflowStep.ts
View file
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
View file
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
View file
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
View file
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 e8933b7

Please sign in to comment.