From 21e73196742439c48f60826e47496224967079e4 Mon Sep 17 00:00:00 2001 From: Miro Yovchev <2827783+myovchev@users.noreply.github.com> Date: Fri, 1 Nov 2024 21:54:56 +0200 Subject: [PATCH 1/6] HMR condition argument and widget player fix --- CHANGELOG.md | 5 +++ modules/@apostrophecms/template/index.js | 24 +++++++---- modules/@apostrophecms/util/ui/src/util.js | 47 ++++++++++++++++++---- 3 files changed, 60 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 208dcf29fc..b0ee600e0d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,11 @@ ### Fixes * Extra bundle detection when using external build module works properly now. +* Widget players are now properly invoked when they arrive later in the page load process. + +### Adds + +* It's possible now to target the HMR build when registering via `template.append` and `template.prepend`. Use `when: 'hmr:public'` or `when: 'hmr:apos'` that will be evaluated against the current asset `options.hmr` configuration. ## 4.9.0 (2024-10-31) diff --git a/modules/@apostrophecms/template/index.js b/modules/@apostrophecms/template/index.js index d7dc024437..2c749a1cb9 100644 --- a/modules/@apostrophecms/template/index.js +++ b/modules/@apostrophecms/template/index.js @@ -864,7 +864,7 @@ module.exports = { // apos.template.prepend({ // component: 'module-name:async-component-name', // where: 'head', - // when: 'hmr, // or e.g. ['hmr', 'dev'], logical AND + // when: 'hmr', // or e.g. ['hmr', 'dev'], logical AND // bundler: 'vite', // }); // OR @@ -909,7 +909,7 @@ module.exports = { // apos.template.append({ // component: 'module-name:async-component-name', // where: 'head', - // when: 'hmr, // or e.g. ['hmr', 'dev'], logical AND + // when: 'hmr', // or e.g. ['hmr', 'dev'], logical AND // bundler: 'vite', // }); // OR @@ -938,6 +938,8 @@ module.exports = { // - `when`: (optional) string or array of strings, the conditions to be met to insert the component. // When an array, a logical AND is applied. One match against the injected `when` data is required. // Currently supported values are `hmr`, `dev`, `prod`. See `getInjectConditionHandlers()` for more info. + // The `when` value can include an argument separated by `:`. E.g. `hmr:apos`, `hmr:public`. If the condition + // handler does not support arguments, it's ignored. // - `bundler`: (optional) string, the alias of the currently registered asset external build module. // The bundler condition is not parth of the actual inject data. It's evaluated just on the registration // data. @@ -975,7 +977,7 @@ module.exports = { conditions.when = conditions.when ? [ conditions.when ] : []; } // Both sides `when` should match - if (data.when && !conditions.when.includes(data.when)) { + if (data.when && !conditions.when.map(s => s.split(':')[0]).includes(data.when)) { return; } if (!data.when && conditions.when.length) { @@ -990,11 +992,12 @@ module.exports = { // `when` being an object same as the schema `if`, supporting // the same logical operators. But it's too much for now. const conditionMet = when.every(val => { - if (!handlers[val]) { + const [ fn, arg ] = val.split(':'); + if (!handlers[fn]) { self.apos.util.error(`Invalid inject condition: ${when}`); return false; } - return handlers[when](data); + return handlers[fn](arg, data); }); if (bundler) { @@ -1020,11 +1023,16 @@ module.exports = { // Simple conditions handling for `when` injects. It can be extended to support // custom conditions in the future - registered by modules similar to // `helpers`. - // Every condition function receives the nunjucks `data` (`when`, `where`, etc) - // object as an argument. + // Every condition function receives an argument if available and the nunjucks + // data object. For example `when: hmr:apos` will call `hmr('apos', data)`. + // The function should return a boolean. getInjectConditionHandlers() { return { - hmr() { + hmr(kind) { + if (kind) { + return self.apos.asset.hasHMR() && + self.apos.asset.getBuildOptions().devServer === kind; + } return self.apos.asset.hasHMR(); }, dev() { diff --git a/modules/@apostrophecms/util/ui/src/util.js b/modules/@apostrophecms/util/ui/src/util.js index 7dc298e765..8bffae2b75 100644 --- a/modules/@apostrophecms/util/ui/src/util.js +++ b/modules/@apostrophecms/util/ui/src/util.js @@ -128,7 +128,20 @@ export default () => { // THAT ONE WIDGET and NO OTHER. Don't worry about finding the // others, we will do that for you and we guarantee only one call per widget. - apos.util.widgetPlayers = {}; + apos.util.widgetPlayersConfig = { + list: {}, + initialized: false + }; + apos.util.widgetPlayers = new Proxy(apos.util.widgetPlayersConfig.list, { + set(target, prop, value) { + target[prop] = value; + // run the player if we missed the initial run + if (apos.util.widgetPlayersConfig.initialized) { + apos.util.runPlayers(document, { [prop]: value }); + } + return true; + } + }); // Run the given function whenever the DOM has new changes that // may require attention. The passed function will be @@ -164,6 +177,23 @@ export default () => { // Alias for onReadyAndRefresh, the recommended way to use and document this functionality apos.util.onReady = apos.util.onReadyAndRefresh.bind(apos.util.onReadyAndRefresh); + // Debounce a function execution. Only the last call within the + // given timeout will be executed. + // Example: + // const debounced = apos.util.debounce((msg) => console.log(msg), 1000); + // debounced('Hello'); + // debounced('World'); + // Only 'World' will be printed after 1 second. + apos.util.debounce = function (fn, timeout) { + let timer; + return (...args) => { + timer && clearTimeout(timer); + timer = setTimeout(() => { + fn(...args); + }, timeout); + }; + }; + // Run all the players that haven't been run. Invoked for you at DOMready // time. You may also invoke it if you just AJAXed in some content and // have reason to suspect there could be widgets in there. You may pass: @@ -184,24 +214,24 @@ export default () => { // DON'T try to find all the widgets. DO just enhance `el`. // This is a computer science principle known as "separation of concerns." - apos.util.runPlayers = function(el) { - const players = apos.util.widgetPlayers; + apos.util.runPlayers = function (el, newPlayer) { + const players = newPlayer || apos.util.widgetPlayers; const playerList = Object.keys(players); for (let i = 0; i < playerList.length; i++) { const playerOpts = players[playerList[i]]; const playerEls = (el || document).querySelectorAll(playerOpts.selector); - playerEls.forEach(function (el) { - if (el.aposWidgetPlayed) { + playerEls.forEach(function (playerEl) { + if (playerEl.aposWidgetPlayed) { return; } // Use an actual property, not a DOM attribute or // "data" prefix property, to avoid the problem of // elements cloned from innerHTML appearing to have // been played too - el.aposWidgetPlayed = true; - playerOpts.player(el); + playerEl.aposWidgetPlayed = true; + playerOpts.player(playerEl); }); } }; @@ -210,7 +240,8 @@ export default () => { // when the page is partially refreshed by the editor. if (!apos.bus) { - apos.util.onReadyAndRefresh(function() { + apos.util.onReady(function () { + apos.util.widgetPlayersConfig.initialized = true; apos.util.runPlayers(); }); } From a364b522c3f8491824f298ace45516a1b0ef29ed Mon Sep 17 00:00:00 2001 From: Miro Yovchev <2827783+myovchev@users.noreply.github.com> Date: Mon, 4 Nov 2024 11:51:51 +0200 Subject: [PATCH 2/6] Make widget list and flag private --- modules/@apostrophecms/util/ui/src/util.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/modules/@apostrophecms/util/ui/src/util.js b/modules/@apostrophecms/util/ui/src/util.js index 8bffae2b75..5f854ccf4a 100644 --- a/modules/@apostrophecms/util/ui/src/util.js +++ b/modules/@apostrophecms/util/ui/src/util.js @@ -128,15 +128,15 @@ export default () => { // THAT ONE WIDGET and NO OTHER. Don't worry about finding the // others, we will do that for you and we guarantee only one call per widget. - apos.util.widgetPlayersConfig = { + const widgetPlayersConfig = { list: {}, initialized: false }; - apos.util.widgetPlayers = new Proxy(apos.util.widgetPlayersConfig.list, { + apos.util.widgetPlayers = new Proxy(widgetPlayersConfig.list, { set(target, prop, value) { target[prop] = value; // run the player if we missed the initial run - if (apos.util.widgetPlayersConfig.initialized) { + if (widgetPlayersConfig.initialized) { apos.util.runPlayers(document, { [prop]: value }); } return true; @@ -241,7 +241,7 @@ export default () => { if (!apos.bus) { apos.util.onReady(function () { - apos.util.widgetPlayersConfig.initialized = true; + widgetPlayersConfig.initialized = true; apos.util.runPlayers(); }); } From e807e5b341dc0b941e706926348b244a7db3b091 Mon Sep 17 00:00:00 2001 From: Miro Yovchev <2827783+myovchev@users.noreply.github.com> Date: Mon, 4 Nov 2024 16:29:58 +0200 Subject: [PATCH 3/6] Remove debounce util --- modules/@apostrophecms/util/ui/src/util.js | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/modules/@apostrophecms/util/ui/src/util.js b/modules/@apostrophecms/util/ui/src/util.js index 5f854ccf4a..cc74cbd1f7 100644 --- a/modules/@apostrophecms/util/ui/src/util.js +++ b/modules/@apostrophecms/util/ui/src/util.js @@ -177,23 +177,6 @@ export default () => { // Alias for onReadyAndRefresh, the recommended way to use and document this functionality apos.util.onReady = apos.util.onReadyAndRefresh.bind(apos.util.onReadyAndRefresh); - // Debounce a function execution. Only the last call within the - // given timeout will be executed. - // Example: - // const debounced = apos.util.debounce((msg) => console.log(msg), 1000); - // debounced('Hello'); - // debounced('World'); - // Only 'World' will be printed after 1 second. - apos.util.debounce = function (fn, timeout) { - let timer; - return (...args) => { - timer && clearTimeout(timer); - timer = setTimeout(() => { - fn(...args); - }, timeout); - }; - }; - // Run all the players that haven't been run. Invoked for you at DOMready // time. You may also invoke it if you just AJAXed in some content and // have reason to suspect there could be widgets in there. You may pass: From c60ecb24e37909dc1eaebbb475611109860091fe Mon Sep 17 00:00:00 2001 From: Miro Yovchev <2827783+myovchev@users.noreply.github.com> Date: Wed, 6 Nov 2024 13:34:12 +0200 Subject: [PATCH 4/6] More restrictive early init guard --- modules/@apostrophecms/util/ui/src/util.js | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/modules/@apostrophecms/util/ui/src/util.js b/modules/@apostrophecms/util/ui/src/util.js index cc74cbd1f7..c2b8ddea04 100644 --- a/modules/@apostrophecms/util/ui/src/util.js +++ b/modules/@apostrophecms/util/ui/src/util.js @@ -137,7 +137,7 @@ export default () => { target[prop] = value; // run the player if we missed the initial run if (widgetPlayersConfig.initialized) { - apos.util.runPlayers(document, { [prop]: value }); + apos.util.runPlayers(null, { init: true }); } return true; } @@ -196,13 +196,25 @@ export default () => { // Your player is guaranteed to run only once per widget. Hint: // DON'T try to find all the widgets. DO just enhance `el`. // This is a computer science principle known as "separation of concerns." + // + // The second argument is an options object. If the `init` option is true, only + // players that haven't already been yet initialized will be run. This option + // is only used internally and shouldn't be passed outside of the core + // initialization process. - apos.util.runPlayers = function (el, newPlayer) { - const players = newPlayer || apos.util.widgetPlayers; + apos.util.runPlayers = function (el, { init = false } = {}) { + const players = apos.util.widgetPlayers; const playerList = Object.keys(players); for (let i = 0; i < playerList.length; i++) { const playerOpts = players[playerList[i]]; + // Guard against multiple player runs early during initialization. + if (init) { + if (playerOpts.initialized) { + continue; + } + playerOpts.initialized = true; + } const playerEls = (el || document).querySelectorAll(playerOpts.selector); playerEls.forEach(function (playerEl) { @@ -225,7 +237,7 @@ export default () => { if (!apos.bus) { apos.util.onReady(function () { widgetPlayersConfig.initialized = true; - apos.util.runPlayers(); + apos.util.runPlayers(null, { init: true }); }); } From 6130b990aec60e8a1bb27f4b31e0af9e929f9452 Mon Sep 17 00:00:00 2001 From: Miro Yovchev <2827783+myovchev@users.noreply.github.com> Date: Wed, 6 Nov 2024 16:12:32 +0200 Subject: [PATCH 5/6] Better option name, more performance --- modules/@apostrophecms/util/ui/src/util.js | 29 +++++++++++----------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/modules/@apostrophecms/util/ui/src/util.js b/modules/@apostrophecms/util/ui/src/util.js index c2b8ddea04..c1931b44a8 100644 --- a/modules/@apostrophecms/util/ui/src/util.js +++ b/modules/@apostrophecms/util/ui/src/util.js @@ -137,7 +137,7 @@ export default () => { target[prop] = value; // run the player if we missed the initial run if (widgetPlayersConfig.initialized) { - apos.util.runPlayers(null, { init: true }); + apos.util.runPlayers(null, { newPlayersOnly: true }); } return true; } @@ -197,24 +197,23 @@ export default () => { // DON'T try to find all the widgets. DO just enhance `el`. // This is a computer science principle known as "separation of concerns." // - // The second argument is an options object. If the `init` option is true, only - // players that haven't already been yet initialized will be run. This option - // is only used internally and shouldn't be passed outside of the core - // initialization process. + // The second argument is an options object. + // If the `newPlayersOnly` option is true, only widget player types whose players + // have not been invoked before will be invoked. This option is used internally + // and shouldn't be needed outside of the core initialization process. - apos.util.runPlayers = function (el, { init = false } = {}) { + apos.util.runPlayers = function (el, { newPlayersOnly = false } = {}) { const players = apos.util.widgetPlayers; - const playerList = Object.keys(players); + let playerList = Object.keys(players); + + // Guard against multiple player runs early during initialization. + if (newPlayersOnly) { + playerList = playerList.filter(player => !players[player].initialized); + playerList.forEach(player => (players[player].initialized = true)); + } for (let i = 0; i < playerList.length; i++) { const playerOpts = players[playerList[i]]; - // Guard against multiple player runs early during initialization. - if (init) { - if (playerOpts.initialized) { - continue; - } - playerOpts.initialized = true; - } const playerEls = (el || document).querySelectorAll(playerOpts.selector); playerEls.forEach(function (playerEl) { @@ -237,7 +236,7 @@ export default () => { if (!apos.bus) { apos.util.onReady(function () { widgetPlayersConfig.initialized = true; - apos.util.runPlayers(null, { init: true }); + apos.util.runPlayers(null, { newPlayersOnly: true }); }); } From 8a1426d652fed697e84577830edea40926cc5655 Mon Sep 17 00:00:00 2001 From: Miro Yovchev <2827783+myovchev@users.noreply.github.com> Date: Wed, 6 Nov 2024 16:24:34 +0200 Subject: [PATCH 6/6] Revert player init checks --- modules/@apostrophecms/util/ui/src/util.js | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/modules/@apostrophecms/util/ui/src/util.js b/modules/@apostrophecms/util/ui/src/util.js index c1931b44a8..68f7074500 100644 --- a/modules/@apostrophecms/util/ui/src/util.js +++ b/modules/@apostrophecms/util/ui/src/util.js @@ -137,7 +137,7 @@ export default () => { target[prop] = value; // run the player if we missed the initial run if (widgetPlayersConfig.initialized) { - apos.util.runPlayers(null, { newPlayersOnly: true }); + apos.util.runPlayers(); } return true; } @@ -196,21 +196,9 @@ export default () => { // Your player is guaranteed to run only once per widget. Hint: // DON'T try to find all the widgets. DO just enhance `el`. // This is a computer science principle known as "separation of concerns." - // - // The second argument is an options object. - // If the `newPlayersOnly` option is true, only widget player types whose players - // have not been invoked before will be invoked. This option is used internally - // and shouldn't be needed outside of the core initialization process. - - apos.util.runPlayers = function (el, { newPlayersOnly = false } = {}) { + apos.util.runPlayers = function (el) { const players = apos.util.widgetPlayers; - let playerList = Object.keys(players); - - // Guard against multiple player runs early during initialization. - if (newPlayersOnly) { - playerList = playerList.filter(player => !players[player].initialized); - playerList.forEach(player => (players[player].initialized = true)); - } + const playerList = Object.keys(players); for (let i = 0; i < playerList.length; i++) { const playerOpts = players[playerList[i]]; @@ -236,7 +224,7 @@ export default () => { if (!apos.bus) { apos.util.onReady(function () { widgetPlayersConfig.initialized = true; - apos.util.runPlayers(null, { newPlayersOnly: true }); + apos.util.runPlayers(); }); }