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

Feature/async blaze #409

Closed
wants to merge 7 commits into from
Closed

Feature/async blaze #409

wants to merge 7 commits into from

Conversation

Grubba27
Copy link
Contributor

@Grubba27 Grubba27 commented Mar 30, 2023

This is a draft PR for people see how is adding async to blaze going.
We have accomplished unwrapping promises in the view.
Below there is a print of the screen + in the PR(will not be in the final version, there is the app that I'm using)
Note that to run it:

cd async-test
meteor npm i
npm run setup
meteor

Screenshot 2023-03-30 at 14 35 53
foo is a result of a promise

  • Add #letAwait to Blaze DSL with simple usage(just unwrap);
    • Tests
  • 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

I've made PR for release 3.0, but I can change this later. Just let me know :)

Closes #364

@StorytellerCZ StorytellerCZ added this to the Blaze 3.0 milestone Mar 31, 2023
@radekmie
Copy link
Collaborator

I've made some progress here:

  • Prepared a sizeable suite of tests in a single template (see screenshot below).
    • No {{#eachAwait}} yet, but it looks like it'd be fairly simple. However, I think I like the truly low-level approach of {{#letAwait}} + {{#each}} more.
  • Cleaned up the non-async implementation, to focus on adding and not changing code.
  • Implemented __async_state__ available only in the {{#letAwait}} blocks. It consists of two fields:
    • error, which is the first rejection or undefined when none was found.
    • pending, which indicates whether any of the promises is still pending.

image

@radekmie
Copy link
Collaborator

Actually, we could handle it on a per-variable basis too:

{{#letAwait x=foo}}
  Resolved value (if any): {{x}}.
  {{#if @pending 'x'}}x is still pending{{else}}x has resolved or rejected{{/if}}
  {{#if @rejected 'x'}}x has rejected {{@rejected 'x'}}{{else}}x has resolved or is still pending{{/if}}
  {{#if @resolved 'x'}}x has resolved {{@resolved 'x'}}{{else}}x has rejected or is still pending{{/if}}
{{/letAwait}}

It can handle general states (i.e., multiple variables) too:

{{#letAwait x=foo y=bar z=baz}}
  Resolved value (if any): {{x}}, {{y}}, {{z}}.
  {{#if @pending}}some binding is still pending{{else}}all bindings have resolved or rejected{{/if}}
  {{#if @rejected}}some binding has rejected{{else}}all bindings have resolved or are still pending{{/if}}
  {{#if @resolved}}some binding has resolved{{else}}all bindings have rejected or are still pending{{/if}}
{{/letAwait}}

Of course these @-helpers can be switched to something like Template., this., or another. The idea is that the #letAwait binds the variables lazily and allows you to query their state. That'd mean we could simply make #let do that, as it'd behave the same for synchronous values (with some performance penalty, of course).

var dataProps = {};
args.forEach(function (arg) {
if (arg.length !== 3) {
// not a keyword arg (x=y)
throw new Error("Incorrect form of #let");
throw new Error("Incorrect form of " + path[0]);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we improve this error message early on? I'd rather have some more characters in the bundle but a clear error message that helps me in debugging when users report them.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay my bad I just saw that all these views have these short error messages. We may discuss this separately.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, all of them do that - I'd keep it as a separate topic.

@radekmie radekmie mentioned this pull request May 2, 2023
Copy link

@DanielDornhardt DanielDornhardt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just wanna say, a great & very clean PR in my opinion! Code-Wise & Logic-Wise it all looks internally consistent & nice.

You're setting up a solid foundation here I'd say. Thank you guys!

@DanielDornhardt
Copy link

Content-wise I have some questions, if that's ok...

If we remember the idea of building another Template helper / operator async, built on top of letAwait:

{{await foo.q.w.e.bar}}

could be turned into this on the compiler / AST level:

<!-- 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}}

This would lead to the desired result being rendered in the template - but the result wouldn't be returned / passed back through the chain as a result value, would it?

Could we access the result of the evaluation? Or would that be a differend / second / separate language construct we'd have to add to spacebars / blaze?

Consider expressions like these:

  1. Blaze "Logic" / helper function calls & using brackets to create sub-expressions like this:
{{#if await someFunc ((await calcParamA) (await someOtherparamB)) (await calcParamB (await someOtherparamC) (someOtherparamD)) }}
    (...)
{{/if}}

For example when used like with the old / oldschool template helpers package:

{{#if $or (await getA (await getParamForA)) (await getB)}}
    (...)
{{/if}}
  1. Attributes also seem to have had a different codepath for destructuring object results

https://www.blazejs.org/guide/spacebars.html#Attribute-Helpers

which kind of threw us for a loop when we were attempting our first async-implementation-attempt ourselves.

So that'd be an important part to check too.

Thank you guys again for your effort and initiative! :)

@radekmie radekmie mentioned this pull request May 5, 2023
4 tasks
@radekmie
Copy link
Collaborator

radekmie commented May 5, 2023

As I'll be taking over this topic, I created #412 with an updated implementation.

@DanielDornhardt I'll address your comments later today or somewhere next week.

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 this pull request may close these issues.

5 participants