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

Allow returning Response from a beforeRequest hook #172

Merged

Conversation

hugomrdias
Copy link
Contributor

@hugomrdias hugomrdias commented Aug 29, 2019

This PR adds support to return a Response from the beforeRequest hook and completely skip the HTTP request.

closes #157

/cc @lidel

@hugomrdias hugomrdias marked this pull request as ready for review August 30, 2019 09:29
@hugomrdias
Copy link
Contributor Author

@sindresorhus can you restart travis please ? the failed build seems to be some random puppeteer timeout.

@sindresorhus
Copy link
Owner

@hugomrdias master is currently failing on Node.js 10 too, so don't worry about that. See #168.

@hugomrdias
Copy link
Contributor Author

cool then this is ready for review

index.js Outdated Show resolved Hide resolved
index.js Outdated
@@ -383,7 +383,10 @@ class Ky {
async _fetch() {
for (const hook of this._hooks.beforeRequest) {
// eslint-disable-next-line no-await-in-loop
await hook(this._input, this._options);
const hookOutput = await hook(this._input, this._options);
Copy link
Collaborator

Choose a reason for hiding this comment

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

hookOutput -> result

readme.md Outdated Show resolved Hide resolved
hugomrdias and others added 3 commits September 9, 2019 15:12
Co-Authored-By: Szymon Marczak <36894700+szmarczak@users.noreply.github.com>
Co-Authored-By: Szymon Marczak <36894700+szmarczak@users.noreply.github.com>
@hugomrdias hugomrdias requested a review from szmarczak September 9, 2019 14:13
@szmarczak szmarczak changed the title feat: add support to return Response from beforeRequest Allow returning Response from a beforeRequest hook Sep 9, 2019
@szmarczak szmarczak merged commit 59d36ac into sindresorhus:master Sep 9, 2019
@hugomrdias hugomrdias deleted the feat/return-response-before-hook branch September 9, 2019 15:04
sindresorhus added a commit that referenced this pull request Sep 9, 2019
@sindresorhus
Copy link
Owner

The docs part should have been added to index.d.ts. Otherwise, looks good.

@donifer
Copy link

donifer commented Oct 2, 2019

I think the types should also be updated 🤔 can't get TS to accept returning a Response

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Skipping request by returning a Response in beforeRequest
4 participants