Add firstRunPromise

[harryadel]

This is a continuation of @DanielDornhardt work, where @radekmie suggestions are applied. I'm unsure whether we should go forward with this considering what was brought up in the slack channel. But I decided to push either way just in case. Ping me please if you want me to undraft it.

[harryadel]

Is there a way for us to make firstRunPromise the default behavior?

Template.onCreated(async function() {
    // autorun 1
    await this.autorun(async(c) => {
        // 1
        await Tracker.withComputation(c, () => { ... }) 
        // 2
        // (...)
    }).firstRunPromise

    // autorun 2
    await this.autorun(async(c) => {
        // 5
        await Tracker.withComputation(c, () => { ... }) 
        // 6
        // (...)
    }).firstRunPromise

    // autorun 3
    await this.autorun(async(c) => {
        // 9
        await Tracker.withComputation(c, () => { ... }) 
        // 10
        // (...)
    }).firstRunPromise
})

Would become:

Template.onCreated(async function() {
    // autorun 1
    await this.autorun(async(c) => {
        // 1
        await Tracker.withComputation(c, () => { ... }) 
        // 2
        // (...)
    })

    // autorun 2
    await this.autorun(async(c) => {
        // 5
        await Tracker.withComputation(c, () => { ... }) 
        // 6
        // (...)
    })

    // autorun 3
    await this.autorun(async(c) => {
        // 9
        await Tracker.withComputation(c, () => { ... }) 
        // 10
        // (...)
    })
})

My reason for this? Predictability! Which is something the original MDG considered when they ran into a similar problem regarding "flush" cycle in general. This is where principle of least astonishment/surpise comes into play. We may also have to implement something akin to flush where it'd allow you to invoke a specific autorun before the others. Can this be done already using invalidate ???

Template.onCreated(async function() {
    // autorun 1
    const firstComputation = await this.autorun(async(c) => {
        // 1
        await Tracker.withComputation(c, () => { ... }) 
        // 2
        // (...)
    })

    // autorun 2
    const secondComputation = await this.autorun(async(c) => {
        // 5
        await Tracker.withComputation(c, () => { ... }) 
        // 6
        // (...)
    })

    // autorun 3
    const thirdComputation = await this.autorun(async(c) => {
        // 9
        await Tracker.withComputation(c, () => { ... }) 
        // 10
        // (...)
    })
    
    // then we do something like this
    // this causes the computations/autoruns to run
    // into the non default way if users may wish to do so for certain reasons
    secondComputation.invalidate();
    firstComputation.invalidate();
    thirdComputation.invalidate();
    // Can we also do Promise.all?
})

tl;dr

My proposal: We come up with a way to allow users to re-order their autoruns if they may wish to do so while keeping the default behavior just like you'd expect where autoruns run in a synchronous looking manner without the need to append firstRunPromise at the end of your autoruns

I've talked with @DanielDornhardt about this and he told me to implement this utility method would be impossible on a technical level as you can't dictate the flow of async once it starts. It's has to do with language level and how JS/async works in general.

Looking forward to hearing your thoughts on this!

---

There was some article which I can't find which talked about POLA, error compilers and something like "programmer pitfall of happiness", can anybody help me find it?

[DanielDornhardt]

Hi @harryadel , funnily enough, this PR is a play on my earlier version of the same thing which worked more like your suggestion, where I introduced autorunAsync which would do exactly what you describe here, return a promise for the computation project.

In General it occurred to me recently, that this feature only goes so far, and I'm actually not 100% sure whether it's that useful or not anymore...

Because often time we (in our project) use this kind of pattern, actually:

this.autorun(() => {
    if (Meteor.loggingIn()) {
        return
    }
    if (!this.subscriptionsReady) {
        return
    }
    // ... do weird stuff :D 
})

Which means that often the first execution of an autorun will return anyhow & then the await for the tracker / the predefined order wouldn't be kept anymore anyhow.

But that means that leaves the original issue be:

// 1st autorun
this.autorun(() => {
    if (Meteor.loggingIn()) {
        return
    }
    if (!this.subscriptionsReady) {
        return
    }
    // ... do weird stuff :D 
})

// 2nd autorun
this.autorun(() => {
    if (Meteor.loggingIn()) {
        return
    }
    if (!this.subscriptionsReady) {
        return
    }
    // ... do weird stuff :D 
})

Before I think the order of execution, once computations got invalidated, would have stayed the same, if the same computations would have changed - that now wouldn't be guaranteed anymore as soon as one await would actually take longer than the other...

And in general: Probably maybe autoruns are actually pretty immune to the order they're executed in, as long as there are no issues of them being triggered multiple times maybe by being async and await'ed now - but I hope that doesn't happen - That's something we should maybe investigate a bit... but in general, maybe this isn't actually as big an issue as I thought it'd be.

What do you other guys think?

[radekmie]

@harryadel

• We can add .then on the Computation object so it'd be awaitable (it still requires something like firstRunPromise but it can remain a private field).
• I didn't get this issue with invalidate - it should work just like it was before.

@DanielDornhardt

• We do a similar thing with useTracker in React components, but we do subscribe in them, so it's more like one autorun with subscribe+if (subscription.ready())+logic.

[DanielDornhardt]

@radekmie

The issue with firstRunPromise is nice but it'll only work for the first run of each autorun...

And if there are eg. a few guard clauses on top of the autorun function, after the first return the firstRunPromise will be resolved.

All following in the autorun await's will be yielding and revisited in quasi-random order throughout the project.

This is especially problematic for eg. method calls with stubs - another one of our old friends I guess:

Async Methods with Callback by denihs · Pull Request #12677 · meteor/meteor · GitHub

async onCreated() {
    await this.autorun(async () => {
        if (await Meteor.loggingInAsync()) {
            return
        }
        const x = await Meteor.callAsync('methodA')
        const y = await Meteor.callAsync('methodB')
        const z = await Meteor.callAsync('methodC')
    }).firstRunPromise

    await this.autorun(async () => {
        if (await Meteor.loggingInAsync()) {
            return
        }
        const x = await Meteor.callAsync('methodD')
        const y = await Meteor.callAsync('methodE')
        const z = await Meteor.callAsync('methodF')
    }).firstRunPromise
}
asyncOnRendered() {
    await this.autorun(async () => {
            if (await Meteor.loggingInAsync()) {
                return
            }
            const x = await Meteor.callAsync('methodX')
            const y = await Meteor.callAsync('methodY')
            const z = await Meteor.callAsync('methodZ')
    }).firstRunPromise
}

This will now lead to methods called in overlapping fashions, and, in case of methods with async stubs on the client, I think they'll complain that they're already in another stub, right?

So... firstRunPromise is nice, but after the first return all bets are off?

[DanielDornhardt]

@radekmie - sorry for just cross-posting here, but it fits in regards to this comment, I don't know where there'd be a better place for these kinds of discussions:

We do a similar thing with useTracker in React components, but we do subscribe in them, so it's more like one autorun with subscribe+if (subscription.ready())+logic.

I thought about another crazy / desperate / stopgap / worst-case-scenario idea: Blaze "suspense hooks" (?):

The difference between React BlazeJS currently is that blaze tries to be clever and only rerenders parts of its template, sub-views, and tries to decide what & when to re-render reactively, from all the different reactive data sources we could use.

If we'd add a function to the template like "observeTheseSubscriptions" and subscribe there and/or pass in certain subscriptions, and anytime there'd be changes (we could add a little debounce of course or something) it'd just redraw itself & all children...

We could "react" based on changes to the minimongo, and not rely on the difficult-to-pass-through-automatically calls to reactive data sources in compination with async/await...

Just one of these ideas... 🤔

[harryadel]

I spoke with Daniel again, and we're trying to get this merged. So, do you have any comments, @radekmie?

The only thing I feel worth considering is this:

We can add .then on the Computation object so it'd be awaitable (it still requires something like firstRunPromise but it can remain a private field).

Do I attempt to add the .then or do we merge this in and iterate on it in another PR?

[radekmie]

OK, so there are two things:

1. The problem where subsequent autoruns are not blocking each other. I understood that the firstRunPromise was needed for initialization, e.g., in Blaze (IIRC, that's where it came from).

   If we're talking about later rerenders, it'd need to schedule whole autoruns, not their parts. That's basically the same problem as with methods and stubs. However, you could try wrapping all of the methods into a block and queue them. Something like this:

   import PQueue from 'p-queue';
   const queue = new PQueue({ concurrency: 1 });
   
   await this.autorun(async () => {
     if (await Meteor.loggingInAsync()) {
         return;
     }
   
     await queue.add(async () => {
       const x = await Meteor.callAsync('methodA');
       const y = await Meteor.callAsync('methodB');
       const z = await Meteor.callAsync('methodC');
     });
   }).firstRunPromise;
   
   await this.autorun(async () => {
     if (await Meteor.loggingInAsync()) {
         return;
     }
   
     await queue.add(async () => {
       const x = await Meteor.callAsync('methodD');
       const y = await Meteor.callAsync('methodE');
       const z = await Meteor.callAsync('methodF');
     });
   }).firstRunPromise;

   (Side note: loggingInAsync? It's a ReactiveVar#get call - there's no point for it to be async.)

2. As for the then method, it should be as simple as:

   class Computation {
     then(onResolved, onRejected) {
       return this.firstRunPromise.then(onResolved, onRejected);
     }
   }

[polygonwood]

The main issue we see in our application is that the lifecycle of Blaze is 'broken' because the order of onCreated on Rendered and onDestroyed is no longer guaranteed : the first async call in an earlier stage fires the next stage.
We had this problem already before the 3.0 story, e.g. with setting up media connections which were already async implemented. The way we solved this is by using a 'runOnce' autorun (forced to run once 'really' because we stop the computation ourselves) by using a reactive variable (typically a Session variable if it's across modules) to hold and release the computation.
E.g. a subset of how we solved that

async onCreated() {
   Session.set('waitFlag',false);
   await SomethingWeHavetoWaitFor();
   Session.set('waitFlag',true);
} 

async onRendered() {
   this.autorun(async (computation) => {
     if (Session.get('flag') {
        await DoWhatHasToBeDoneAfterDOMReady();
      // if needed set another reactive varibale to kick helpers e.g.
       computation.stop()
    }
})
}

Perhaps a Blaze version aware of async lifecylces (because it's not always required) could have some standardized solution for this ?

We have used similar approaches to e.g. dynamic subscriptions (if it can not be solved by the Router).

Maybe this can inspire someone ?

`

[Grubba27]

Looks good but I would ask making what @radekmie suggested in the Computation object, adding a .then property so we could use this like this:

await Tracker.autorun(async () => {
  await Meteor.userAsync();
  (...more async code...)
})

[radekmie]

Sorry for a late review!

[Grubba27]

Maybe remove this comment? we are already explaining this in the paragraph above.
Add examples that are not using .firstRunPromise as well.

[StorytellerCZ]

automagically, 🤔 I really like this word. Probably a typo here, but I think this should be something we should strive for in Meteor.

[Grubba27]

It occurs only two times in our code base, and it does not follow that much how we write our docs, but I agree. We could review our documentation guide line and make some adjustments to be more pleasing to read.

I like this writing in docs(Puns and funny comments) generally, but I need to maintain how it is today.

[harryadel]

It's not a typo. I meant to use it since Meteor is all about that magic. Pffffft you guys won't get it!

[StorytellerCZ]

Looks like this is ready to be merged.

[harryadel]

@StorytellerCZ BRING THE "ta-da" AND "automagically" BACK

@Grubba27 You label yourself as an "open sourcerer" and you can't even do no automagic? YOU'RE A PHONY!

[Grubba27]

lol