Skip to content

fix(response): use never return type for abort method #13

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

Merged
merged 2 commits into from
Dec 23, 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: 2 additions & 1 deletion adonis-typings/response.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,9 @@ declare module '@ioc:Adonis/Core/Response' {
plainCookie (key: string, value: any, options?: Partial<CookieOptions>): this
clearCookie (key: string): this

abort (body: any, status?: number): void
abort (body: any, status?: number): never
abortIf (conditional: any, body: any, status?: number): void
abortUnless (conditional: any, body: any, status?: number): asserts conditional

finish (): void
}
Expand Down
12 changes: 11 additions & 1 deletion src/Response/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -819,7 +819,7 @@ export class Response extends Macroable implements ResponseContract {
* Abort the request with custom body and a status code. 400 is
* used when status is not defined
*/
public abort (body: any, status?: number): void {
public abort (body: any, status?: number): never {
throw HttpException.invoke(body, status || 400)
}

Expand All @@ -833,6 +833,16 @@ export class Response extends Macroable implements ResponseContract {
}
}

/**
* Abort the request with custom body and a status code when
* passed condition returns `false`
*/
public abortUnless (condition: any, body: any, status?: number): asserts condition {
if (!condition) {
this.abort(body, status)
}
}

/**
* Set signed cookie as the response header. The inline options overrides
* all options from the config (means they are not merged).
Expand Down
37 changes: 35 additions & 2 deletions test/response.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1034,7 +1034,7 @@ test.group('Response', (group) => {
assert.deepEqual(body, { message: 'Not allowed' })
})

test('abort request when condition is truthy', async (assert) => {
test('abortIf: abort request when condition is truthy', async (assert) => {
const server = createServer((req, res) => {
const config = fakeConfig()
const response = new Response(req, res, config)
Expand All @@ -1051,7 +1051,7 @@ test.group('Response', (group) => {
assert.deepEqual(body, { message: 'Not allowed' })
})

test('do not abort request when condition is falsy', async () => {
test('abortIf: do not abort request when condition is falsy', async () => {
const server = createServer((req, res) => {
const config = fakeConfig()
const response = new Response(req, res, config)
Expand All @@ -1066,4 +1066,37 @@ test.group('Response', (group) => {

await supertest(server).get('/').expect(200)
})

test('abortUnless: abort request when condition is falsy', async (assert) => {
const server = createServer((req, res) => {
const config = fakeConfig()
const response: Response = new Response(req, res, config)
Copy link
Member Author

Choose a reason for hiding this comment

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

See microsoft/TypeScript#33622

It is necessary here to explicitly declare the type of Response. In practice in Adonis, the type is already defined in the context so the user doesn't have to do anything.

Example with the change, where TS correctly determined that the code after the call is unreachable:
image

try {
response.abortUnless(false, { message: 'Not allowed' }, 401)
} catch (error) {
error.handle(error, { response })
}

response.finish()
})

const { body } = await supertest(server).get('/').expect(401)
assert.deepEqual(body, { message: 'Not allowed' })
})

test('abortUnless: do not abort request when condition is truthy', async () => {
const server = createServer((req, res) => {
const config = fakeConfig()
const response: Response = new Response(req, res, config)
try {
response.abortUnless(true, { message: 'Not allowed' }, 401)
} catch (error) {
error.handle(error, { response })
}

response.finish()
})

await supertest(server).get('/').expect(200)
})
})