Added support for Promises in #if and #each.

[radekmie]

In this pull request, I follow up on #412 and #413 by supporting Promises in #if/#unless and #each. The idea is basically the same as how it works in #let except for where we actually store the ReactiveVars.

To do:

• #if/#unless and #each are not rendering anything before the Promise resolves. That's to make it clear that there's nothing there. I'm open for suggestions though, as I can imagine cases where {{else}} is more expected.
  • I asked a couple of people and they said it's fine not to render anything as in "not to make any decisions before the value is known".
  • Due to the complexity, #each will treat unresolved Promises as empty sequences and thus render the {{else}} block. We may change that in the future.
• #if/#unless and #each are not rendering anything for rejected Promises. This is a sane choice, but I want to confirm that with the community.
  • I asked a couple of people and they said it's fine not to render anything as in "not to make any decisions before the value is known".
  • Due to the complexity, #each will treat rejected Promises as empty sequences and thus render the {{else}} block. We may change that in the future.
• Tests.
• Documentation.
• Decide whether or not we want to preserve the if.__conditionVar and each.argVar. They're not used anywhere, but sometimes it makes sense (e.g., #with depends on that).
  • I decided to leave them for now, but there's quite a few potential simplifications in this area. They may improve the performance as well.

Note: #428 got merged into this branch!

[DanielDornhardt]

Hi, i have some issues with our autoform forms not showing with this patch, previous version from Meteor 2.12 worked. I'll try to investigate what is happening.

Below is a screenshot of an error message I get, albeit only after a reload / HCP:

[jankapunkt]

@DanielDornhardt this is likely an Autoform issue. If so please open in Autoform and we continue to investigate there

[StorytellerCZ]

Would this be for Blaze 3 or 2?

[DanielDornhardt]

Hi @jankapunkt I mean it's probably autoform related, but as this is a change in the backwards compatibility of blaze it could potentially concern other code too I think.

Also this is not yet in a meteor release, so should we make it an issue in autoform when actually the issue isn't with autoform but with the blaze change?

Of course it'd make sense to investigate & see if there's a simple fix, I'm all for pragmatism here. But I just don't know whether this'd be the way to go?

-> I'll make a small repo with a replication of the issue and share it here & in the autoform repo.

@StorytellerCZ : It'd probably be for Blaze 2 too, as it'd be required for people to get their projects ready for 3 I guess.

[DanielDornhardt]

Hi, i've made a small replication & also a loom video for a quick impression. I'll also cross-post in the autoform repository in case someone wants to help to identify & fix this issue.

Loom Video: https://www.loom.com/share/a202e704da8c4600a386c5ece7e5a287?sid=cbfcff77-30fe-4832-9185-df37baf3af5e

Link to the repo: https://github.com/trusted-care/autoform-sandbox-blaze-async

[DanielDornhardt]

Hi, I think I found the issue @radekmie : #unless is functionally identical with #if, meaning it's not the inverse anymore, which of course leads to all kinds of issues in blaze applications & libraries, including Autoform...

Here's a short demo:

https://www.loom.com/share/8c5e5e4fa12e4858859c38d925e1c04a?sid=55c827d8-d0a5-422c-ae62-e9214678d981

[radekmie]

Yeah, I see that it's not really unless now 😅 I'll push a fix for that soon and will take care of the rest this or next week.

[DanielDornhardt]

Thank you @radekmie I'll check it out! :)

[DanielDornhardt]

Lookin' better! :D Thank you a lot! We'll see how far this will get us!

[radekmie]

This PR is done! I had to change the logic for #each and render {{else}} for unresolved and rejected Promises, as it'd complicate the entire thing much more otherwise.

I still think this is entirely backward-compatible for non-weird cases (i.e., #if promise) and can be released as a minor version. That's the same logic we went with for #let changes.

[sebastianspiller]

Hey @radekmie ,

I tried "if" in our project (I am a collegue of Daniel) and added it inside an html attribute like

<a class="{{#if isTruthyWithPromise}}arrow{{/if}}">

This is not working and the browser is running on very high load.

[radekmie]

@sebastianspiller I looked into it and unfortunately it's not that simple. The reason is the way Blaze handle attributes. Let's make an example:

<template name="spacebars_async_tests_attributes">
  <img id="{{#if x}}1{{else}}0{{/if}}">
</template>

Translates to:

Template.__checkName("spacebars_async_tests_attributes");
Template["spacebars_async_tests_attributes"] = new Template("Template.spacebars_async_tests_attributes", (function() {
  var view = this;
  return HTML.IMG({
    id: function() {
      return Blaze.If(function() {
        return Spacebars.call(view.lookup("x"));
      }, function() {
        return "1";
      }, function() {
        return "0";
      })}
  });
}));

The culprit is that id is a function that creates a new Blaze.If every time - after its creation, it gets rendered once and destroyed (see Blaze._expandView). It's terribly inefficient but most importantly - stateless. And Blaze.If needs to be stateful now. So every time Blaze.If resolves its condition, it gets updated, destroyed and recreated - hence the loop and high CPU usage.

I tried to work around it, but there's no easy way around it. I'd even argue there's no way to fix it without touching the compiler and changing the way attributes are handled. And that's a lot.

[DanielDornhardt]

Hi...

I mean - the if's in attributes thing - I think that warrants some thinking... two things pop into my mind:

a) if that's an infinite loop, that probably should be prevented / the user should be warned if that's not supported
b) it also raises the question of the documentation & the consistency when async data sources are used within blaze templates - it'd be good to have a guide later to say "this stuff works, here are some workarounds we figured out, and these things aren't supported" in order for people to get a clear understanding of how it works. Eg here: https://www.blazejs.org/api/spacebars#In-Attribute-Values it doesn't warn users (yet) that this will crash the browser, we should update the docs after we're done.

c) maybe a + b can make an efficient developer rack his brain a bit whether a small compiler hack couldn't be warranted in order to avoid the busywork of both a) and b) 😄

[radekmie]

I agree that for now, a detection + clear error would be needed for it to work. In the future, it could be improved by somehow marking the attribute as async and for example wrap it with #let.

[DanielDornhardt]

Mmh... sounds nice... especially this part:

In the future, it could be improved by somehow marking the attribute as async and for example wrap it with #let.

😄

A wise guy said something like this in the beginning of the year: "We could probably handle that on the AST level probably, by working with the AST it should be possible to replace the call.chain.resolution within blaze with a series of #lets"...

Is there a possibility in this direction here too?

[radekmie]

I just pushed some Promise detection and a couple of tests for that, so it won't silently loop forever. I also updated the docs.

@jankapunkt, @StorytellerCZ, @Grubba27 I consider this one done.

[StorytellerCZ]

LGTM, what do you think @jankapunkt ?

[Grubba27]

Could not we target the devel(v2.x) branch for this PR? This does not look to me as a breaking change.

[DanielDornhardt]

Having this in 2.x / 2.14 for example would definitely help people prep for Meteor 3.0 as they could start transitioning earlier and wouldn't have to convert everything all at once once they try to migrate to 3.0...

And yes, for whether this'd be a breaking change, we're running with this patch for a few months now and it looks good.

This PR: [https://github.com//pull/428](Implemented async attributes and content. #428) should also go into 2.x / 2.14 I think because all these things are the things which help people to prepare their projects now instead of having to wait for a stable Meteor 3.0 and having to do everything then.

[radekmie]

Sure, I'll rebase it.