From a08373c5e60f9fde9e8268e5ba748b4f76d3c0b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Miernik?= Date: Sat, 13 Jan 2024 15:53:04 +0100 Subject: [PATCH 1/3] Implemented async dynamic attributes (closes #443). --- packages/blaze/materializer.js | 38 ++++++++++--- packages/htmljs/visitors.js | 10 ++-- packages/spacebars-tests/async_tests.html | 4 ++ packages/spacebars-tests/async_tests.js | 67 +++++++++++++++-------- site/source/api/spacebars.md | 12 ++-- 5 files changed, 91 insertions(+), 40 deletions(-) diff --git a/packages/blaze/materializer.js b/packages/blaze/materializer.js index b1f2182b2..5404626fb 100644 --- a/packages/blaze/materializer.js +++ b/packages/blaze/materializer.js @@ -95,7 +95,32 @@ const materializeDOMInner = function (htmljs, intoArray, parentView, workStack) const isPromiseLike = x => !!x && typeof x.then === 'function'; -function waitForAllAttributesAndContinue(attrs, fn) { +function then(maybePromise, fn) { + if (isPromiseLike(maybePromise)) { + maybePromise.then(fn); + } else { + fn(maybePromise); + } +} + +function waitForAllAttributes(attrs) { + if (attrs !== Object(attrs)) { + console.log(attrs) + return attrs; + } + + // Combined attributes, e.g., ``. + if (Array.isArray(attrs)) { + const mapped = attrs.map(waitForAllAttributes); + return mapped.some(isPromiseLike) ? Promise.all(mapped) : mapped; + } + + // Singular async attributes, e.g., ``. + if (isPromiseLike(attrs)) { + return attrs.then(waitForAllAttributes); + } + + // Singular sync attributes, with potentially async properties. const promises = []; for (const [key, value] of Object.entries(attrs)) { if (isPromiseLike(value)) { @@ -113,11 +138,8 @@ function waitForAllAttributesAndContinue(attrs, fn) { } } - if (promises.length) { - Promise.all(promises).then(fn); - } else { - fn(); - } + // If any of the properties were async, lift the `Promise`. + return promises.length ? Promise.all(promises).then(() => attrs) : attrs; } const materializeTag = function (tag, parentView, workStack) { @@ -156,8 +178,8 @@ const materializeTag = function (tag, parentView, workStack) { const attrUpdater = new ElementAttributesUpdater(elem); const updateAttributes = function () { const expandedAttrs = Blaze._expandAttributes(rawAttrs, parentView); - waitForAllAttributesAndContinue(expandedAttrs, () => { - const flattenedAttrs = HTML.flattenAttributes(expandedAttrs); + then(waitForAllAttributes(expandedAttrs), awaitedAttrs => { + const flattenedAttrs = HTML.flattenAttributes(awaitedAttrs); const stringAttrs = {}; Object.keys(flattenedAttrs).forEach((attrName) => { // map `null`, `undefined`, and `false` to null, which is important diff --git a/packages/htmljs/visitors.js b/packages/htmljs/visitors.js index 51e6fd2f5..f846eaf0a 100644 --- a/packages/htmljs/visitors.js +++ b/packages/htmljs/visitors.js @@ -10,6 +10,7 @@ import { isVoidElement, } from './html'; +const isPromiseLike = x => !!x && typeof x.then === 'function'; var IDENTITY = function (x) { return x; }; @@ -156,6 +157,11 @@ TransformingVisitor.def({ // an array, or in some uses, a foreign object (such as // a template tag). visitAttributes: function (attrs, ...args) { + // Allow Promise-like values here; these will be handled in materializer. + if (isPromiseLike(attrs)) { + return attrs; + } + if (isArray(attrs)) { var result = attrs; for (var i = 0; i < attrs.length; i++) { @@ -172,10 +178,6 @@ TransformingVisitor.def({ } if (attrs && isConstructedObject(attrs)) { - if (typeof attrs.then === 'function') { - throw new Error('Asynchronous dynamic attributes are not supported. Use #let to unwrap them first.'); - } - throw new Error("The basic TransformingVisitor does not support " + "foreign objects in attributes. Define a custom " + "visitAttributes for this case."); diff --git a/packages/spacebars-tests/async_tests.html b/packages/spacebars-tests/async_tests.html index 8aca4ba62..4f8f8641d 100644 --- a/packages/spacebars-tests/async_tests.html +++ b/packages/spacebars-tests/async_tests.html @@ -76,6 +76,10 @@ + + diff --git a/packages/spacebars-tests/async_tests.js b/packages/spacebars-tests/async_tests.js index 8a328beb7..4f859076d 100644 --- a/packages/spacebars-tests/async_tests.js +++ b/packages/spacebars-tests/async_tests.js @@ -22,20 +22,20 @@ function asyncSuite(templateName, cases) { } } -const getter = async () => 'foo'; -const thenable = { then: resolve => Promise.resolve().then(() => resolve('foo')) }; -const value = Promise.resolve('foo'); +const getter = v => async () => v; +const thenable = v => ({ then: resolve => Promise.resolve().then(() => resolve(v)) }); +const value = v => Promise.resolve(v); asyncSuite('access', [ - ['getter', { x: { y: getter } }, '', 'foo'], - ['thenable', { x: { y: thenable } }, '', 'foo'], - ['value', { x: { y: value } }, '', 'foo'], + ['getter', { x: { y: getter('foo') } }, '', 'foo'], + ['thenable', { x: { y: thenable('foo') } }, '', 'foo'], + ['value', { x: { y: value('foo') } }, '', 'foo'], ]); asyncSuite('direct', [ - ['getter', { x: getter }, '', 'foo'], - ['thenable', { x: thenable }, '', 'foo'], - ['value', { x: value }, '', 'foo'], + ['getter', { x: getter('foo') }, '', 'foo'], + ['thenable', { x: thenable('foo') }, '', 'foo'], + ['value', { x: value('foo') }, '', 'foo'], ]); asyncTest('missing1', 'outer', async (test, template, render) => { @@ -49,27 +49,48 @@ asyncTest('missing2', 'inner', async (test, template, render) => { }); asyncSuite('attribute', [ - ['getter', { x: getter }, '', ''], - ['thenable', { x: thenable }, '', ''], - ['value', { x: value }, '', ''], + ['getter', { x: getter('foo') }, '', ''], + ['thenable', { x: thenable('foo') }, '', ''], + ['value', { x: value('foo') }, '', ''], ]); -asyncTest('attributes', '', async (test, template, render) => { - Blaze._throwNextException = true; - template.helpers({ x: Promise.resolve() }); - test.throws(render, 'Asynchronous dynamic attributes are not supported. Use #let to unwrap them first.'); -}); +asyncSuite('attributes', [ + ['getter in getter', { x: getter({ class: getter('foo') }) }, '', ''], // Nested getters are NOT evaluated. + ['getter in thenable', { x: thenable({ class: getter('foo') }) }, '', ''], // Nested getters are NOT evaluated. + ['getter in value', { x: value({ class: getter('foo') }) }, '', ''], // Nested getters are NOT evaluated. + ['static in getter', { x: getter({ class: 'foo' }) }, '', ''], + ['static in thenable', { x: thenable({ class: 'foo' }) }, '', ''], + ['static in value', { x: value({ class: 'foo' }) }, '', ''], + ['thenable in getter', { x: getter({ class: thenable('foo') }) }, '', ''], + ['thenable in thenable', { x: thenable({ class: thenable('foo') }) }, '', ''], + ['thenable in value', { x: value({ class: thenable('foo') }) }, '', ''], + ['value in getter', { x: getter({ class: value('foo') }) }, '', ''], + ['value in thenable', { x: thenable({ class: value('foo') }) }, '', ''], + ['value in value', { x: value({ class: value('foo') }) }, '', ''], +]); + +asyncSuite('attributes_double', [ + ['null lhs getter', { x: getter({ class: null }), y: getter({ class: 'foo' }) }, '', ''], + ['null lhs thenable', { x: thenable({ class: null }), y: thenable({ class: 'foo' }) }, '', ''], + ['null lhs value', { x: value({ class: null }), y: value({ class: 'foo' }) }, '', ''], + ['null rhs getter', { x: getter({ class: 'foo' }), y: getter({ class: null }) }, '', ''], + ['null rhs thenable', { x: thenable({ class: 'foo' }), y: thenable({ class: null }) }, '', ''], + ['null rhs value', { x: value({ class: 'foo' }), y: value({ class: null }) }, '', ''], + ['override getter', { x: getter({ class: 'foo' }), y: getter({ class: 'bar' }) }, '', ''], + ['override thenable', { x: thenable({ class: 'foo' }), y: thenable({ class: 'bar' }) }, '', ''], + ['override value', { x: value({ class: 'foo' }), y: value({ class: 'bar' }) }, '', ''], +]); asyncSuite('value_direct', [ - ['getter', { x: getter }, '', 'foo'], - ['thenable', { x: thenable }, '', 'foo'], - ['value', { x: value }, '', 'foo'], + ['getter', { x: getter('foo') }, '', 'foo'], + ['thenable', { x: thenable('foo') }, '', 'foo'], + ['value', { x: value('foo') }, '', 'foo'], ]); asyncSuite('value_raw', [ - ['getter', { x: getter }, '', 'foo'], - ['thenable', { x: thenable }, '', 'foo'], - ['value', { x: value }, '', 'foo'], + ['getter', { x: getter('foo') }, '', 'foo'], + ['thenable', { x: thenable('foo') }, '', 'foo'], + ['value', { x: value('foo') }, '', 'foo'], ]); asyncSuite('if', [ diff --git a/site/source/api/spacebars.md b/site/source/api/spacebars.md index 7a9fd9188..e59f322a1 100644 --- a/site/source/api/spacebars.md +++ b/site/source/api/spacebars.md @@ -228,7 +228,7 @@ and value strings. For convenience, the value may also be a string or null. An empty string or null expands to `{}`. A non-empty string must be an attribute name, and expands to an attribute with an empty value; for example, `"checked"` expands to `{checked: ""}` (which, as far as HTML is concerned, means the -checkbox is checked). `Promise`s are not supported and will throw an error. +checkbox is checked). To summarize: @@ -242,10 +242,6 @@ To summarize: {checked: "", 'class': "foo"}checked class=foo {checked: false, 'class': "foo"}class=foo "checked class=foo"ERROR, string is not an attribute name - - Promise.resolve({}) - ERROR, asynchronous dynamic attributes are not supported, see #443 - @@ -262,6 +258,12 @@ specifies a value for the `class` attribute, it will overwrite `{% raw %}{{myCla As always, Spacebars takes care of recalculating the element's attributes if any of `myClass`, `attrs1`, or `attrs2` changes reactively. +### Async Dynamic Attributes + +The dynamic attributes can be wrapped in a `Promise`. When that happens, they +will be treated as `undefined` while it's pending or rejected. Once resolved, +the resulting value is used. To have more fine-grained handling of non-resolved +states, use `#let` and the async state helpers (e.g., `@pending`). ## Triple-braced Tags From b626929595f20cb6e7bfdd67a4100433520bb474 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Miernik?= Date: Sun, 14 Jan 2024 12:00:29 +0100 Subject: [PATCH 2/3] Updated handling on non-object dynamic attributes. --- packages/blaze/materializer.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/blaze/materializer.js b/packages/blaze/materializer.js index 5404626fb..cc22ee921 100644 --- a/packages/blaze/materializer.js +++ b/packages/blaze/materializer.js @@ -104,9 +104,9 @@ function then(maybePromise, fn) { } function waitForAllAttributes(attrs) { - if (attrs !== Object(attrs)) { - console.log(attrs) - return attrs; + // Non-object attrs (e.g., `null`) are ignored. + if (!attrs || attrs !== Object(attrs)) { + return {}; } // Combined attributes, e.g., ``. From a6ebfe4438b001ef685280f0818f0d7247b38fdc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Miernik?= Date: Wed, 24 Jan 2024 22:07:27 +0100 Subject: [PATCH 3/3] Implemented error logging for all async actions. --- packages/blaze/exceptions.js | 7 +++++++ packages/blaze/materializer.js | 12 +++++++----- packages/spacebars/spacebars-runtime.js | 14 ++++++++------ 3 files changed, 22 insertions(+), 11 deletions(-) diff --git a/packages/blaze/exceptions.js b/packages/blaze/exceptions.js index 2e62a8e67..b99ce4c95 100644 --- a/packages/blaze/exceptions.js +++ b/packages/blaze/exceptions.js @@ -42,6 +42,13 @@ Blaze._reportException = function (e, msg) { debugFunc()(msg || 'Exception caught in template:', e.stack || e.message || e); }; +// It's meant to be used in `Promise` chains to report the error while not +// "swallowing" it (i.e., the chain will still reject). +Blaze._reportExceptionAndThrow = function (error) { + Blaze._reportException(error); + throw error; +}; + Blaze._wrapCatchingExceptions = function (f, where) { if (typeof f !== 'function') return f; diff --git a/packages/blaze/materializer.js b/packages/blaze/materializer.js index cc22ee921..d81547f13 100644 --- a/packages/blaze/materializer.js +++ b/packages/blaze/materializer.js @@ -97,7 +97,7 @@ const isPromiseLike = x => !!x && typeof x.then === 'function'; function then(maybePromise, fn) { if (isPromiseLike(maybePromise)) { - maybePromise.then(fn); + maybePromise.then(fn, Blaze._reportException); } else { fn(maybePromise); } @@ -117,7 +117,7 @@ function waitForAllAttributes(attrs) { // Singular async attributes, e.g., ``. if (isPromiseLike(attrs)) { - return attrs.then(waitForAllAttributes); + return attrs.then(waitForAllAttributes, Blaze._reportExceptionAndThrow); } // Singular sync attributes, with potentially async properties. @@ -126,20 +126,22 @@ function waitForAllAttributes(attrs) { if (isPromiseLike(value)) { promises.push(value.then(value => { attrs[key] = value; - })); + }, Blaze._reportExceptionAndThrow)); } else if (Array.isArray(value)) { value.forEach((element, index) => { if (isPromiseLike(element)) { promises.push(element.then(element => { value[index] = element; - })); + }, Blaze._reportExceptionAndThrow)); } }); } } // If any of the properties were async, lift the `Promise`. - return promises.length ? Promise.all(promises).then(() => attrs) : attrs; + return promises.length + ? Promise.all(promises).then(() => attrs, Blaze._reportExceptionAndThrow) + : attrs; } const materializeTag = function (tag, parentView, workStack) { diff --git a/packages/spacebars/spacebars-runtime.js b/packages/spacebars/spacebars-runtime.js index 9f7f958b2..3bc3b993f 100644 --- a/packages/spacebars/spacebars-runtime.js +++ b/packages/spacebars/spacebars-runtime.js @@ -131,12 +131,14 @@ Spacebars.makeRaw = function (value) { function _thenWithContext(promise, fn) { const computation = Tracker.currentComputation; const view = Blaze.currentView; - return promise.then(value => - Blaze._withCurrentView(view, () => - Tracker.withComputation(computation, () => - fn(value) - ) - ) + return promise.then( + value => + Blaze._withCurrentView(view, () => + Tracker.withComputation(computation, () => + fn(value) + ) + ), + Blaze._reportExceptionAndThrow ); }