add: make log filter async friendly by adding logAsync #3141
Conversation
Right now the `log` filter logs the passed in values directly. This isn't always helpful when working with async values e.g. in webc components. This change adds the `logAsync` filter to also take promises as any value and awaits them, so you always get some useful output. It could be possible to just eagerly return the value in the sync filter and then use a `.then` call to log the values, but that would leave room for unexpected data mutation, so I didn't implement it that way for now. Signed-off-by: Raphael Höser <[email protected]>
|
Can we test for promises in |
|
If we make the log filter itself async, it would no longer be available in languages that don't support async filters (like handlebars). I've just seen that addFilter only adds the filters to Liquid, JavaScriptFunctions and Nunjucks filters, so we could make it indeed async for everything. |
Signed-off-by: Raphael Höser <[email protected]>
|
Ah, sorry—I meant (kind of) like this config.addFilter("log", (...messages) => {
// if any of messages are a promise, do promise things
if(messages.find(entry => /* promise test */)) {
// out of order, but that’s preferable to errors in non-async
Promise.all(messages).then(msgs => console.log(...msgs));
} else {
console.log(...messages);
}
});some prior art (not exactly): Line 272 in d77d7fa |
|
Ahh okay, I see what you mean and I will adapt it. |
|
Thinking about it, we could even return a promise if any of the inputs is async. That would also fix the ordering issue for async cases or am I missing something here? |
|
@zachleat are we intending to support only native Promises here, or also all "thenables" (so e.g. Bluebird promises and similar custom promise implementations)? |
|
native promises only is fine. And I like the promise return! That wouldn’t work with our async filtering stuff though, since it checks if the callback is async (not the return value). |
|
@zachleat Good catch. Since the removal of some of the built-in templating languages, is there a reason to not just add it via |
Right now the
logfilter logs the passed in values directly. This isn't always helpful when working with async values e.g. in webc components. This change adds thelogAsyncfilter to also take promises as any value and awaits them, so you always get some useful output.It could be possible to just eagerly return the value in the sync filter and then use a
.thencall to log the values, but that would leave room for unexpected data mutation, so I didn't implement it that way for now.resolves #3133