From fa85f8a121b4c0bfa11fe16df1670c3093f30a96 Mon Sep 17 00:00:00 2001 From: Richard Walker Date: Tue, 12 Nov 2024 18:53:11 +1300 Subject: [PATCH 1/5] fix: refactor hints to assets This should be classed as a breaking change, however, since hints has thus far been experimental and has not been used outside of various internal HTTP streaming tests, I'm claiming that this can be a non breaking change. --- lib/assets.js | 47 +++++++++++++++++++++++++ lib/hints.js | 47 ------------------------- lib/http-incoming.js | 6 ++-- tests/{hints.test.js => assets.test.js} | 28 +++++++-------- tests/http-incoming.test.js | 18 ++++++---- 5 files changed, 76 insertions(+), 70 deletions(-) create mode 100644 lib/assets.js delete mode 100644 lib/hints.js rename tests/{hints.test.js => assets.test.js} (56%) diff --git a/lib/assets.js b/lib/assets.js new file mode 100644 index 0000000..8faa8f5 --- /dev/null +++ b/lib/assets.js @@ -0,0 +1,47 @@ +import { EventEmitter } from 'node:events'; + +export class Assets extends EventEmitter { + #expectedAssets = new Set(); + #receivedAssets = new Map(); + + // pointless constructor with super to appease TS which cryptically errors otherwise. + constructor() { + super(); + } + + /** + * Adds a new expected asset to the set + * Expected assets are used to determine if all assets have been received + * @param {string} name - The name of the asset + */ + addExpectedAsset(name) { + this.#expectedAssets.add(name); + } + + /** + * Adds a new received asset to the set + * After each asset has been received, we check if all assets have been received + * and emit the 'complete' event if so + * @param {string} name - The name of the asset + * @param {{js: Array, css: Array}} assets - The assets for a given resource + */ + addReceivedAsset(name, assets = { js: [], css: [] }) { + this.#receivedAssets.set(name, assets); + if (this.allAssetsReceived) { + const assets = Array.from(this.#receivedAssets.values()); + const js = assets.flatMap(({ js = [] }) => js).filter(Boolean); + const css = assets.flatMap(({ css = [] }) => css).filter(Boolean); + this.emit('received', { js, css }); + } + } + + /** + * Value will be true if all expected assets have been received + */ + get allAssetsReceived() { + return ( + this.#expectedAssets.size === this.#receivedAssets.size && + [...this.#expectedAssets].every((x) => this.#receivedAssets.has(x)) + ); + } +} diff --git a/lib/hints.js b/lib/hints.js deleted file mode 100644 index dc0956b..0000000 --- a/lib/hints.js +++ /dev/null @@ -1,47 +0,0 @@ -import { EventEmitter } from 'node:events'; - -export class Hints extends EventEmitter { - #expectedHints = new Set(); - #receivedHints = new Map(); - - // pointless constructor with super to appease TS which cryptically errors otherwise. - constructor() { - super(); - } - - /** - * Adds a new expected hint to the set - * Expected hints are used to determine if all hints have been received - * @param {string} name - The name of the hint - */ - addExpectedHint(name) { - this.#expectedHints.add(name); - } - - /** - * Adds a new received hint to the set - * After each hint has been received, we check if all hints have been received - * and emit the 'complete' event if so - * @param {string} name - The name of the hint - * @param {{js: Array, css: Array}} assets - The hinted assets for a given resource - */ - addReceivedHint(name, assets = { js: [], css: [] }) { - this.#receivedHints.set(name, assets); - if (this.allHintsReceived) { - const assets = Array.from(this.#receivedHints.values()); - const js = assets.flatMap(({ js = [] }) => js).filter(Boolean); - const css = assets.flatMap(({ css = [] }) => css).filter(Boolean); - this.emit('complete', { js, css }); - } - } - - /** - * Value will be true if all expected hints have been received - */ - get allHintsReceived() { - return ( - this.#expectedHints.size === this.#receivedHints.size && - [...this.#expectedHints].every((x) => this.#receivedHints.has(x)) - ); - } -} diff --git a/lib/http-incoming.js b/lib/http-incoming.js index f1121d1..324c735 100644 --- a/lib/http-incoming.js +++ b/lib/http-incoming.js @@ -1,5 +1,5 @@ import { URL } from 'node:url'; -import { Hints } from './hints.js'; +import { Assets } from './assets.js'; const inspect = Symbol.for('nodejs.util.inspect.custom'); @@ -59,8 +59,8 @@ export default class HttpIncoming { */ #js; - /** @type {Hints} */ - hints = new Hints(); + /** @type {Assets} */ + assets = new Assets(); /** * @constructor diff --git a/tests/hints.test.js b/tests/assets.test.js similarity index 56% rename from tests/hints.test.js rename to tests/assets.test.js index b533ed5..4e61937 100644 --- a/tests/hints.test.js +++ b/tests/assets.test.js @@ -1,26 +1,26 @@ import tap from 'tap'; -import { Hints } from '../lib/hints.js'; +import { Assets } from '../lib/assets.js'; tap.test('Hints() - allHintsReceived property - should be true', (t) => { - const hints = new Hints(); - hints.addExpectedHint('foo'); - hints.addExpectedHint('bar'); - hints.addReceivedHint('foo', { + const assets = new Assets(); + assets.addExpectedAsset('foo'); + assets.addExpectedAsset('bar'); + assets.addReceivedAsset('foo', { js: [{ value: 'foo.js' }], css: [{ value: 'foo.css' }], }); - hints.addReceivedHint('bar', { + assets.addReceivedAsset('bar', { js: [{ value: 'foo.js' }], css: [{ value: 'foo.css' }], }); - t.equal(hints.allHintsReceived, true); + t.equal(assets.allAssetsReceived, true); t.end(); }); -tap.test('Hints() - complete event - should fire', (t) => { +tap.test('Hints() - received event - should fire', (t) => { t.plan(6); - const hints = new Hints(); - hints.on('complete', ({ js, css }) => { + const assets = new Assets(); + assets.on('received', ({ js, css }) => { t.equal(js.length, 2); t.equal(css.length, 2); t.equal(js[0].value, 'foo.js'); @@ -29,13 +29,13 @@ tap.test('Hints() - complete event - should fire', (t) => { t.equal(css[1].value, 'foo.css'); t.end(); }); - hints.addExpectedHint('foo'); - hints.addExpectedHint('bar'); - hints.addReceivedHint('foo', { + assets.addExpectedAsset('foo'); + assets.addExpectedAsset('bar'); + assets.addReceivedAsset('foo', { js: [{ value: 'foo.js' }], css: [{ value: 'foo.css' }], }); - hints.addReceivedHint('bar', { + assets.addReceivedAsset('bar', { js: [{ value: 'foo.js' }], css: [{ value: 'foo.css' }], }); diff --git a/tests/http-incoming.test.js b/tests/http-incoming.test.js index 1a08e0e..26d845e 100644 --- a/tests/http-incoming.test.js +++ b/tests/http-incoming.test.js @@ -367,14 +367,20 @@ tap.test('can read values from view with default types', (t) => { * @type {HttpIncoming} */ const incoming = new HttpIncoming(SIMPLE_REQ, SIMPLE_RES); - incoming.hints.addExpectedHint('foo'); - incoming.hints.addExpectedHint('bar'); + incoming.assets.addExpectedAsset('foo'); + incoming.assets.addExpectedAsset('bar'); - incoming.hints.on('complete', () => { - t.ok(incoming.hints.allHintsReceived); + incoming.assets.on('received', () => { + t.ok(incoming.assets.allAssetsReceived); t.end(); }); - incoming.hints.addReceivedHint('foo'); - incoming.hints.addReceivedHint('bar'); + incoming.assets.addReceivedAsset('foo', { + js: [{ value: 'foo.js' }], + css: [{ value: 'foo.css' }], + }); + incoming.assets.addReceivedAsset('bar', { + js: [{ value: 'foo.js' }], + css: [{ value: 'foo.css' }], + }); }); From c4aaaf2cf90880d2c69a0ae1577b1676fc5e31ff Mon Sep 17 00:00:00 2001 From: Richard Walker Date: Tue, 12 Nov 2024 19:29:49 +1300 Subject: [PATCH 2/5] feat: add waitForAssets method --- lib/assets.js | 16 ++++++++--- lib/http-incoming.js | 19 +++++++++++++ tests/http-incoming.test.js | 54 ++++++++++++++++++++++++++++++++++--- 3 files changed, 83 insertions(+), 6 deletions(-) diff --git a/lib/assets.js b/lib/assets.js index 8faa8f5..9bbcb6e 100644 --- a/lib/assets.js +++ b/lib/assets.js @@ -3,6 +3,8 @@ import { EventEmitter } from 'node:events'; export class Assets extends EventEmitter { #expectedAssets = new Set(); #receivedAssets = new Map(); + #js; + #css; // pointless constructor with super to appease TS which cryptically errors otherwise. constructor() { @@ -29,12 +31,20 @@ export class Assets extends EventEmitter { this.#receivedAssets.set(name, assets); if (this.allAssetsReceived) { const assets = Array.from(this.#receivedAssets.values()); - const js = assets.flatMap(({ js = [] }) => js).filter(Boolean); - const css = assets.flatMap(({ css = [] }) => css).filter(Boolean); - this.emit('received', { js, css }); + this.#js = assets.flatMap(({ js = [] }) => js).filter(Boolean); + this.#css = assets.flatMap(({ css = [] }) => css).filter(Boolean); + this.emit('received', { js: this.#js, css: this.#css }); } } + get js() { + return this.#js; + } + + get css() { + return this.#css; + } + /** * Value will be true if all expected assets have been received */ diff --git a/lib/http-incoming.js b/lib/http-incoming.js index 324c735..03c0bbb 100644 --- a/lib/http-incoming.js +++ b/lib/http-incoming.js @@ -222,6 +222,25 @@ export default class HttpIncoming { return this.#js; } + /** + * Wait for assets to be received from + * @returns {Promise<{js: Array, css: Array}>} + */ + waitForAssets() { + if (this.assets.allAssetsReceived) { + return Promise.resolve({ + js: this.assets.js, + css: this.assets.css, + }); + } + + return new Promise((resolve) => { + this.assets.on('received', (assets) => { + resolve(assets); + }); + }); + } + /** * @returns {PodiumHttpIncoming} */ diff --git a/tests/http-incoming.test.js b/tests/http-incoming.test.js index 26d845e..978e977 100644 --- a/tests/http-incoming.test.js +++ b/tests/http-incoming.test.js @@ -359,10 +359,8 @@ tap.test('can read values from view with default types', (t) => { t.end(); }); -tap.test('can read values from view with default types', (t) => { +tap.test('can set asset expectations and receive assets', (t) => { t.plan(1); - // really only here for tsc - /** * @type {HttpIncoming} */ @@ -384,3 +382,53 @@ tap.test('can read values from view with default types', (t) => { css: [{ value: 'foo.css' }], }); }); + +tap.test('can wait for expected assets', (t) => { + t.plan(1); + /** + * @type {HttpIncoming} + */ + const incoming = new HttpIncoming(SIMPLE_REQ, SIMPLE_RES); + incoming.assets.addExpectedAsset('foo'); + incoming.assets.addExpectedAsset('bar'); + + incoming.waitForAssets().then(() => { + t.ok(incoming.assets.allAssetsReceived); + t.end(); + }); + + incoming.assets.addReceivedAsset('foo', { + js: [{ value: 'foo.js' }], + css: [{ value: 'foo.css' }], + }); + incoming.assets.addReceivedAsset('bar', { + js: [{ value: 'foo.js' }], + css: [{ value: 'foo.css' }], + }); +}); + +tap.test('waitForAssets will resolve even if all assets have already been received', async (t) => { + t.plan(3); + /** + * @type {HttpIncoming} + */ + const incoming = new HttpIncoming(SIMPLE_REQ, SIMPLE_RES); + incoming.assets.addExpectedAsset('foo'); + incoming.assets.addExpectedAsset('bar'); + + incoming.assets.addReceivedAsset('foo', { + js: [{ value: 'foo.js' }], + css: [{ value: 'foo.css' }], + }); + incoming.assets.addReceivedAsset('bar', { + js: [{ value: 'foo.js' }], + css: [{ value: 'foo.css' }], + }); + + const { js, css } = await incoming.waitForAssets(); + + t.ok(incoming.assets.allAssetsReceived); + t.same(js, [{ value: 'foo.js' }, { value: 'foo.js' }]); + t.same(css, [{ value: 'foo.css' }, { value: 'foo.css' }]); + t.end(); +}); From 34ff0f78e4763be5fce6249ae9ca197a167ef0f8 Mon Sep 17 00:00:00 2001 From: Richard Walker Date: Tue, 12 Nov 2024 19:34:43 +1300 Subject: [PATCH 3/5] chore: fix code comment --- lib/http-incoming.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/http-incoming.js b/lib/http-incoming.js index 03c0bbb..0746892 100644 --- a/lib/http-incoming.js +++ b/lib/http-incoming.js @@ -223,7 +223,8 @@ export default class HttpIncoming { } /** - * Wait for assets to be received from + * Wait for assets to be received from podlets + * * @returns {Promise<{js: Array, css: Array}>} */ waitForAssets() { From 8c9ec781e8fe72eaec6271bf39a1306e2894d105 Mon Sep 17 00:00:00 2001 From: Richard Walker Date: Tue, 12 Nov 2024 19:36:43 +1300 Subject: [PATCH 4/5] chore: fix lint issues --- tests/http-incoming.test.js | 47 ++++++++++++++++++++----------------- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/tests/http-incoming.test.js b/tests/http-incoming.test.js index 978e977..245cd50 100644 --- a/tests/http-incoming.test.js +++ b/tests/http-incoming.test.js @@ -407,28 +407,31 @@ tap.test('can wait for expected assets', (t) => { }); }); -tap.test('waitForAssets will resolve even if all assets have already been received', async (t) => { - t.plan(3); - /** - * @type {HttpIncoming} - */ - const incoming = new HttpIncoming(SIMPLE_REQ, SIMPLE_RES); - incoming.assets.addExpectedAsset('foo'); - incoming.assets.addExpectedAsset('bar'); +tap.test( + 'waitForAssets will resolve even if all assets have already been received', + async (t) => { + t.plan(3); + /** + * @type {HttpIncoming} + */ + const incoming = new HttpIncoming(SIMPLE_REQ, SIMPLE_RES); + incoming.assets.addExpectedAsset('foo'); + incoming.assets.addExpectedAsset('bar'); - incoming.assets.addReceivedAsset('foo', { - js: [{ value: 'foo.js' }], - css: [{ value: 'foo.css' }], - }); - incoming.assets.addReceivedAsset('bar', { - js: [{ value: 'foo.js' }], - css: [{ value: 'foo.css' }], - }); + incoming.assets.addReceivedAsset('foo', { + js: [{ value: 'foo.js' }], + css: [{ value: 'foo.css' }], + }); + incoming.assets.addReceivedAsset('bar', { + js: [{ value: 'foo.js' }], + css: [{ value: 'foo.css' }], + }); - const { js, css } = await incoming.waitForAssets(); + const { js, css } = await incoming.waitForAssets(); - t.ok(incoming.assets.allAssetsReceived); - t.same(js, [{ value: 'foo.js' }, { value: 'foo.js' }]); - t.same(css, [{ value: 'foo.css' }, { value: 'foo.css' }]); - t.end(); -}); + t.ok(incoming.assets.allAssetsReceived); + t.same(js, [{ value: 'foo.js' }, { value: 'foo.js' }]); + t.same(css, [{ value: 'foo.css' }, { value: 'foo.css' }]); + t.end(); + }, +); From e16819c41de495a239b60ee2bb0dc77ce033f3ac Mon Sep 17 00:00:00 2001 From: Richard Walker Date: Wed, 13 Nov 2024 13:58:04 +1300 Subject: [PATCH 5/5] refactor: remove unneeded boolean check --- lib/assets.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/assets.js b/lib/assets.js index 9bbcb6e..1c04a8a 100644 --- a/lib/assets.js +++ b/lib/assets.js @@ -31,8 +31,8 @@ export class Assets extends EventEmitter { this.#receivedAssets.set(name, assets); if (this.allAssetsReceived) { const assets = Array.from(this.#receivedAssets.values()); - this.#js = assets.flatMap(({ js = [] }) => js).filter(Boolean); - this.#css = assets.flatMap(({ css = [] }) => css).filter(Boolean); + this.#js = assets.flatMap(({ js = [] }) => js); + this.#css = assets.flatMap(({ css = [] }) => css); this.emit('received', { js: this.#js, css: this.#css }); } }