Skip to content

Commit ab3b448

Browse files
Peejadavidlehn
authored andcommitted
Defer importing ky-universal until actually needed
This accomplishes two things: * It prevents a dangling promise with an import in it. If the module is loaded (queuing up the import promise) and then no clients are ever used, and thus never awaited, the import can happen too late. In particular, Jest will break if the import happens after the test is complete. * It avoids a dynamic import altogether when the client isn't needed. In particular, Jest uses Node's VM API which only experimentally supports dynamic imports (and ESM modules altogether). This change means that merely loading the http-client module doesn't require enabling that experimental support.
1 parent 728fc8e commit ab3b448

File tree

3 files changed

+71
-4
lines changed

3 files changed

+71
-4
lines changed

lib/deferred.js

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
export function deferred(f) {
2+
let promise;
3+
4+
return {
5+
then(
6+
onfulfilled,
7+
onrejected
8+
) {
9+
promise ||= new Promise(resolve => resolve(f()));
10+
return promise.then(
11+
onfulfilled,
12+
onrejected
13+
);
14+
},
15+
};
16+
}

lib/httpClient.js

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,10 @@
22
* Copyright (c) 2020-2022 Digital Bazaar, Inc. All rights reserved.
33
*/
44
import {convertAgent} from './agentCompatibility.js';
5+
import {deferred} from './deferred.js';
56

6-
export const kyOriginalPromise = import('ky-universal')
7-
.then(({default: ky}) => ky);
7+
export const kyOriginalPromise = deferred(() => import('ky-universal')
8+
.then(({default: ky}) => ky));
89

910
export const DEFAULT_HEADERS = {
1011
Accept: 'application/ld+json, application/json'
@@ -33,7 +34,7 @@ export function createInstance({
3334
params = convertAgent(params);
3435

3536
// create new ky instance that will asynchronously resolve
36-
const kyPromise = parent.then(kyBase => {
37+
const kyPromise = deferred(() => parent.then(kyBase => {
3738
let ky;
3839
if(parent === kyOriginalPromise) {
3940
// ensure default headers, allow overrides
@@ -46,7 +47,7 @@ export function createInstance({
4647
ky = kyBase.extend({headers, ...params});
4748
}
4849
return ky;
49-
});
50+
}));
5051

5152
return _createHttpClient(kyPromise);
5253
}

tests/deferred.spec.js

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
import {deferred} from '../lib/deferred.js';
2+
3+
describe('deferred()', () => {
4+
it('resolves to the return value of its function', async () => {
5+
const d = deferred(() => {
6+
return 'return value';
7+
});
8+
9+
const ret = await d;
10+
ret.should.equal('return value');
11+
});
12+
13+
it('defers execution until awaited', async () => {
14+
let executionCount = 0;
15+
executionCount.should.equal(0);
16+
17+
const d = deferred(() => {
18+
executionCount++;
19+
return 'return value';
20+
});
21+
22+
executionCount.should.equal(0);
23+
await d;
24+
executionCount.should.equal(1);
25+
});
26+
27+
it('only executes once', async () => {
28+
let executionCount = 0;
29+
executionCount.should.equal(0);
30+
31+
const d = deferred(() => {
32+
executionCount++;
33+
return 'return value';
34+
});
35+
36+
await d;
37+
await d;
38+
executionCount.should.equal(1);
39+
});
40+
41+
it('unwraps returned promises', async () => {
42+
const d = deferred(() => {
43+
return Promise.resolve('return value');
44+
});
45+
46+
const ret = await d;
47+
ret.should.equal('return value');
48+
});
49+
});
50+

0 commit comments

Comments
 (0)