This repository has been archived by the owner on Mar 10, 2020. It is now read-only.
fix: increase default timeout and respect value passed to ky.extend
#1130
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some of our operations take a really long time, if we don't set a timeout for `ky` we get the default of 10 seconds. This PR sets the timeout to `false` if one is not explicitly passed which disables it. Nb. I had to add the default to `false` to every invocation. Looking at the code it should be enough to do it in `src/lib/configure.js` but it doesn't seem to be.
|
Predictably it's because we pass |
achingbrain
force-pushed
the
disable-timeout-if-not-set
branch
from
df2a7e3
to
cc40ce5
Compare
|
I've changed the timeout to wait for something stupid but not to wait forever, and also added a Depends on schnittstabil/merge-options#14 |
achingbrain
changed the title
ky.extend
achingbrain
force-pushed
the
disable-timeout-if-not-set
branch
from
0d42bf4
to
af66a0a
Compare
Comment on lines
+73
to
+77
| function wrap (fn, defaults) { | ||
| return (input, options) => { | ||
| return fn(input, mergeOptions(defaults, options)) | ||
| } | ||
| } |
There was a problem hiding this comment.
Suggestion:
const wrap = (fn, defaults) => (input, options) => fn(input, mergeOptions(defaults, options));There was a problem hiding this comment.
I find this sort of deeply chained single-line-arrow-function style optimises readability for computers and not humans. It'll get minified to something similar to this anyway so there's little to gain here.
hugomrdias
approved these changes
Oct 20, 2019
achingbrain
added a commit
that referenced
this pull request
Oct 21, 2019
…#1130) * fix: disable timeout if not set Some of our operations take a really long time, if we don't set a timeout for `ky` we get the default of 10 seconds. This PR sets the timeout to `false` if one is not explicitly passed which disables it. Nb. I had to add the default to `false` to every invocation. Looking at the code it should be enough to do it in `src/lib/configure.js` but it doesn't seem to be. * fix: use `ignoreUndefined` merge-options option * chore: update bundle size * chore: remove git url
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Some of our operations take a really long time, if we don't set a timeout for
kywe get the default of 10 seconds. This PR sets the timeout tofalseif one is not explicitly passed which disables it.Nb. I had to add the default to
falseto every invocation. Looking at the code it should be enough to do it insrc/lib/configure.jsbut it doesn't seem to be. Might be a bug inky, I'll dig a little deeper.