Skip to content

Commit

Permalink
Defer importing ky-universal until actually needed
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Peeja authored and davidlehn committed Apr 14, 2023
1 parent 728fc8e commit ab3b448
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 4 deletions.
16 changes: 16 additions & 0 deletions lib/deferred.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
export function deferred(f) {
let promise;

return {
then(
onfulfilled,
onrejected
) {
promise ||= new Promise(resolve => resolve(f()));
return promise.then(
onfulfilled,
onrejected
);
},
};
}
9 changes: 5 additions & 4 deletions lib/httpClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@
* Copyright (c) 2020-2022 Digital Bazaar, Inc. All rights reserved.
*/
import {convertAgent} from './agentCompatibility.js';
import {deferred} from './deferred.js';

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

export const DEFAULT_HEADERS = {
Accept: 'application/ld+json, application/json'
Expand Down Expand Up @@ -33,7 +34,7 @@ export function createInstance({
params = convertAgent(params);

// create new ky instance that will asynchronously resolve
const kyPromise = parent.then(kyBase => {
const kyPromise = deferred(() => parent.then(kyBase => {
let ky;
if(parent === kyOriginalPromise) {
// ensure default headers, allow overrides
Expand All @@ -46,7 +47,7 @@ export function createInstance({
ky = kyBase.extend({headers, ...params});
}
return ky;
});
}));

return _createHttpClient(kyPromise);
}
Expand Down
50 changes: 50 additions & 0 deletions tests/deferred.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import {deferred} from '../lib/deferred.js';

describe('deferred()', () => {
it('resolves to the return value of its function', async () => {
const d = deferred(() => {
return 'return value';
});

const ret = await d;
ret.should.equal('return value');
});

it('defers execution until awaited', async () => {
let executionCount = 0;
executionCount.should.equal(0);

const d = deferred(() => {
executionCount++;
return 'return value';
});

executionCount.should.equal(0);
await d;
executionCount.should.equal(1);
});

it('only executes once', async () => {
let executionCount = 0;
executionCount.should.equal(0);

const d = deferred(() => {
executionCount++;
return 'return value';
});

await d;
await d;
executionCount.should.equal(1);
});

it('unwraps returned promises', async () => {
const d = deferred(() => {
return Promise.resolve('return value');
});

const ret = await d;
ret.should.equal('return value');
});
});

0 comments on commit ab3b448

Please sign in to comment.