Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle async code #364

Open
jankapunkt opened this issue Mar 28, 2022 · 20 comments · Fixed by #412
Open

Handle async code #364

jankapunkt opened this issue Mar 28, 2022 · 20 comments · Fixed by #412
Milestone

Comments

@jankapunkt
Copy link
Collaborator

Blaze currently auto-calls functions:

Template.example.helpers({
  asyncTest () {
    return {
      foo: () => 'bar'
    }
  }
})
<template name="example">
  {{#with asyncTest}}{{this.foo}}{{/with}}
</template>

will lead to bar as output.

However, changing foo to an async function like so

Template.example.helpers({
  asyncTest () {
    return {
      foo: async () => 'bar'
    }
  }
})

we will get for the same Template a toString output of a pending Promise: [object Promise]

However, we should be able to handle Promises in 2022 in any context. This would imply a huge PR but I think this is a crucial candidate for Blaze 3 and the future of Blaze

@StorytellerCZ StorytellerCZ added this to the Blaze 3.0 milestone Mar 28, 2022
@StorytellerCZ
Copy link
Collaborator

Agreed!
I'm thinking that both Meteor and Blaze v3 will be all about Fibers and Promises. I think things will move towards the same pattern of development and support as Node has.

@StorytellerCZ
Copy link
Collaborator

OK, I think this is something that we should include as a main feature in Blaze 2.7.
Sadly I'm currently unable to in any way participate in building this functionality.

@jankapunkt
Copy link
Collaborator Author

Maybe we can first collect what all needs to be supported, especially in combination with all new changes by Meteor 2.8+

@klablink
Copy link

Regarding asynchronous management, I think it is also useful to consider hooks: created, rendered and destroyed. Now callbacks are synchronous but we will often have asynchronous calls within them.

@klablink
Copy link

FYI: I tried implementing the Tracker context logic, currentComputation, with zone.js but it doesn't not intercept the native promises (async/await). Used with legacy version of the bundle it works but feels like going backwards. Does anyone know of alternative packages to do this?

@Grubba27
Copy link
Contributor

My point of view regarding handling async code would be divided in two parts. Not sure if it makes 100% sense but it could be a way to implement this.
First we need to handle promises/promise-like functions. We could do something like svelte, as I commented in the other pr or other ways.

{#await promise}
	<p>...waiting</p>
{:then number}
	<p>The number is {number}</p>
{:catch error}
	<p style="color: red">{error.message}</p>
{/await}

in blaze could be like:

<template name="example">
 {{#with asyncTest}}
    {{#await this.foo}}
	<p>...waiting</p>
    {{:then bar}} 
	<p>result: {bar}</p>
    {{:catch error}}
	<p style="color: red">{error.message}</p>
    {{/await }}
 {{/with}}
</template>

Then or in parallel, we would need to handle how to make Meteor/tracker async / accept async dependencies. As for today it has its limitations regarding async

@klablink
Copy link

The idea is not bad, but Tracker has to support asynchronous processes and to do that I think it will have to track calls, a bit like Angular and Dart. Your Blaze example can already be done with reactive variables and a heavy memory overhead, but if we have no way to intercept changes to an asynchronous object we lose reactivity. Assume the following pseudo-example

const vars = new ReactiveVar();
Tracker.autorun(async () => {
  const user = await Meteor.userAsync(); 
  const a = await coll.find({userCode: user.profile.companyCode).fetchAsync();
  const b = await coll2.find({extCode: a.extCode).fetchAsync();
  vars.set(b):
}) 

Changes coming from the DDP are not intercepted because on the first await the autorun loses context. Losing the native Meteor reactivity has a big impact.

My opinion is to keep Minimongo client-side synchronous at least until the Tracker problem is solved.

@polygonwood
Copy link

The concept of synchronous Minimongo access is something I suggested also in the main Meteor Async PR discussion: async focus can go first to Meteor.call use cases. It would reduce impact to smaller set of Tracker async cases one has to deal with.
In my opinion, finding a solution for the predictability of the onCreated, onRendered, onDestroyed sequence, and the concept of helpers kicking in after onCreated only, is a more impacting challenge to address.
If one could be sure that a OnCreatedAsync hook would be waited for before the OnRendered or OnRenderedAsync hook is called, that could make it easy to refactor these hooks to using async back end calls.
The next thing could be await-ed event hooks, because these are also the trivial places to find async back end calls. Helpers are conceptually already more or less async since they fire once and by reactiveness.

@polygonwood
Copy link

Changes coming from the DDP are not intercepted because on the first await the autorun loses context. Losing the native Meteor reactivity has a big impact.

@klablink I observed this (indirectly) by turning a helper into an async helper, can you elaborate why the context is lost after the first await ?

My opinion is to keep Minimongo client-side synchronous at least until the Tracker problem is solved.

Agreed !

@klablink
Copy link

klablink commented Jan 9, 2023

@klablink I observed this (indirectly) by turning a helper into an async helper, can you elaborate why the context is lost after the first await?

Hi @polygonwood

Blaze's reactivity is based on Meteor's Tracker package. This uses a global variable, currentComputation, to map the relationship between a function to be called when a certain reactive variable changes and/or a minimon search. Now, if the process becomes asynchronous, on the first await the currentComputation will lose the reference for the rest of the calls.

For better understanding I will insert the code extract of Current Tracker:

_compute() {
    this.invalidated = false;

    var previous = Tracker.currentComputation;
    setCurrentComputation(this);
    var previousInCompute = inCompute;
    inCompute = true;
    try {
      withNoYieldsAllowed(this._func)(this);
    } finally {
      setCurrentComputation(previous);
      inCompute = previousInCompute;
    }
  }

As you can see, this._func must be synchronous to work in the current context, otherwise setCurrentComputation(previous) is executed immediately, losing context.

There has been much discussion on the subject (see this PR), and it seems that in Meteor version 2.10 there will be the possibility of invoking asynchronous functions by passing the tracker context. This will mean a major change for Blaze and many client packages.

@polygonwood
Copy link

polygonwood commented Jan 10, 2023 via email

@sebastianspiller
Copy link

Hello from Germany 😃

I saw Tracker.withComputation in 2.10 and had a look on blaze, how hard can it be to make it work? 😅

I could quickly asynchronize some functions and I could write some self resolving helpers in a demo project, I was enthusiastic about it and wanted more 😬, when I saw no stringified [object Promise], but when I started to dive deeper, everyhing became async, like Blaze.renderWithData, maybe I did s.th. wrong and one false placed async has a huge consequence on the whole code base.

At the moment, async pops up everywhere, what I don't want because then the whole API will be async, too, in future. I have problems now, that the Blaze.View is missing.

I will keep on and just wanted to mention, that someone's working on it and want to know, if someone else gives it a try at the moment. Simply putting async everywhere doesn't seem the solution and I may think of another approach.

Regards 💯

@Grubba27
Copy link
Contributor

Probably, to make this happen, you will need to use .then other than async, IMO I would make this a two-step approach, one keyword like withSync to get the promise and then unwarp it, that is the second step, where it could resolve to a value that can be stringified and in sync. Or it can resolve to an error that can also be stringified.
It would look a lot like svelte but is the way with the best DX

@sebastianspiller
Copy link

sebastianspiller commented Jan 26, 2023

Hi, I made some progress, talked to @jankapunkt and want to share a code snippet:

view.js : 350
I added a reactive var to Blaze.View and in the Blaze._materializeView function, I did this:

let htmljs = view._render()

htmljs = view.promiseReactiveVar.get() ? view.promiseReactiveVar.get() : htmljs

if (htmljs instanceof Promise) {
  htmljs.then(val => {
    view.promiseReactiveVar.set(val)
  })
  return
}

With this trick, htmljs will trigger an autorun as soon as its Promise resolves. So I could work inside Spacbars like Spacebars.dot and make them async, so that htmljs can handle the Promises.

I ran the test suite and only 2 failed, but they also fail since 2.10, so my next steps are:

  • Work with new ES6 updated PRs
  • Update blaze test-app
  • Write Tests for async stuff

Problems are, that I couldn't manage to make this works so that Attributes

work, but with my approach, it might be, that you can do the following:
{{asyncHelper.text}} or {{asyncCollectionHelper.method.text}}

I'll keep you uptodate 😃

@Grubba27
Copy link
Contributor

Grubba27 commented Mar 6, 2023

Hey @sebastianspiller, how is it going? Maybe you could share your changes so that we may check these tests as well.
By having a PR based on your fork, we could be helping on another front, such as the testing and documenting

@Grubba27
Copy link
Contributor

Grubba27 commented Mar 23, 2023

We had a meeting today with the blaze-async-workgroup and we came up with the first task / concept for blaze.
Daniel D or @sebastianspiller will submit the PR with the helpers being async and we will tackle next the following(idealized by @radekmie):

<!-- This is a basic building block -->

{{#letAwait x=foo.q.w.e.r.t.y.bar y=baz}}
  I can use {{x}} here and {{y}}, but `foo.q.w.e.r.t.y.bar` has to be a promise.
  None of the prefix path (`foo`, `foo.q`, `foo.q.w`, etc.) cannot!
{{pending}}
  Here either `x` or `y` is still pending.
{{rejected}}
  Here either `x` or `y` is rejected.
{{/letAwait}}

<!-- End-user API (ideally) -->
<!-- We'd need to have a way of adding a `{{pending}}` and `{{rejected}}` blocks. -->
<template name="example">
  <button className="js-click">Click me</button>
  {{#if userClickedMeEvenNumberOfTimes}}
    {{await foo.q.w.e.bar}}
  {{/if}}
</template>

<!-- Desugared (ideally) -->
{{#if userClickedMeEvenNumberOfTimes}}
  {{#letAwait temp1=foo}}
    {{#letAwait temp2=temp1.q}}
      {{#letAwait temp3=temp2.w}}
        {{#letAwait temp4=temp3.e}}
          {{#letAwait temp5=temp4.bar}}
            {{temp5}}
          {{/letAwait}}
        {{/letAwait}}
      {{/letAwait}}
    {{/letAwait}}
  {{/letAwait}}
{{/if}}

for the tasks, what we have is:

  • Add #letAwait to Blaze DSL with simple usage(just unwrap) (PR with tests and such);
  • Work on having pending and error states for #letAwait
  • Work in the building block such as {{await something}} or {{await s.foo}}
  • Work on having pending and error states for await keyword, maybe a #await block for this

In these tasks, the latter three can be parallelized, but we need the letAwait building block

@zodern
Copy link
Member

zodern commented Mar 24, 2023

I like the syntax for this. Is there a reason it is named #letAwait instead of #await?

One problem with svelte and its await blocks is every time the promise changes, it removes the DOM for the await block, creates the DOM for the pending block, and then when the promise resolves removes the DOM for the pending block and recreates the DOM for the await block. In most situations that is fine, but for minimonogo queries with results that frequently change or that have DOM that is expensive to render, this would be very inefficient since every time the results change it creates a new promise that immediately resolves. For this and other reasons, I'm considering recommending svelte apps not use await blocks for queries.

What is the plan to avoid this performance problem in blaze?

@radekmie
Copy link
Collaborator

Is there a reason it is named #letAwait instead of #await?

I just wanted to make it stay in-align with #let. await (or #await, but it's not a block) is also a good name.

[...] What is the plan to avoid this performance problem in blaze?

There's non... Yet. I've spent on it about an hour now, just making sure what is possible syntax-wise (including parser). Maybe a Suspense-like approach from React would work? That is, schedule updates "once in a while" and adjust DOM only if the update won't make it.

But then again, it's a separate issue - Minimongo queries (or at least some of them) could rely on observe, just like useFind in React integration (source).

@radekmie
Copy link
Collaborator

radekmie commented May 2, 2023

#409 should be linked here.

@Grubba27 Grubba27 mentioned this issue May 2, 2023
5 tasks
@radekmie radekmie linked a pull request May 5, 2023 that will close this issue
4 tasks
@Grubba27
Copy link
Contributor

Grubba27 commented Jul 6, 2023

I think with #412 we can close this one @jankapunkt ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants