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

tag to handle streams #313

Open
mlrawlings opened this issue Jun 20, 2016 · 21 comments
Open

tag to handle streams #313

mlrawlings opened this issue Jun 20, 2016 · 21 comments
Labels

Comments

@mlrawlings
Copy link
Member

mlrawlings commented Jun 20, 2016

@patrick-steele-idem and I have discussed adding a tag to marko's core that handles streaming in an elegant way.

The syntax that we're looking at is the <on> tag:

<ul>
    <on(user in data.usersStream)>
        <li>${user.name}</li>  
    </on>
</ul>

We'd also support a more verbose syntax for generic EventEmitters:

<on('data' as user in usersStream until 'end')>

Note: <for(x in stream)> was considered, but it would complicate the looping logic of regular for loops and require the stream logic to be present even for templates that never make use of it. We need something that is syntactically different so we can distinguish at compile time.

Note 2: we also considered <for-async(x in stream)> and <for await(x in stream)> however <on> has the benefit of making sense when used with any EventEmitter.

In many ways, the <on> tag will be similar to the <await> tag (previously <async-fragment>), however the <on> tag will also flush multiple times throughout it's rendering cycle.

There is a tradeoff to consider around flushing. If we flush after each iteration, content will be pushed to the browser sooner, but if we buffer a few iterations and gzip is enabled, the resulting payload will be smaller. We need a good default for flushing and an easy, intuitive way to configure it.

We also need to figure out what should be provided by the tag, and what interesting use cases we want to support. One that comes to mind immediately is infinite scrolling.

All that said, here is my proposed set of attributes for the <on> tag:

  • @var - parsed from the argument, the variable to be used inside the body of the tag
  • @event - parsed from the argument, the name of the event to listen to, defaults to "data"
  • @endEvent - parsed from the argument, the name of the event that signifies the end of the loop, defaults to "end"
  • @data-provider - parsed from the argument, the stream or emitter we're listening to
  • @name - the name of the <on> tag. used for debugging and the show-after attribute
  • FUTURE: @client-reorder - the <on> tag will not block the rest of the page, JS is required on client
  • FUTURE: @show-after - when client-reorder is set to true then displaying this instance's content will be delayed until the referenced <await> or <on> instance is shown.
  • @buffer-count - how many iterations of the <on> tag to buffer before flushing
  • @buffer-duration - if the time since the last flush is greater than the buffer duration, then the <on> tag will flush its buffer, regardless of the current buffer count
  • @timeout - if the time since the last chunk of data was emitted is greater than the timeout, then the <on> tag will timeout
  • @total-timeout - if the time since the rendering started is greater than the total timeout, then the <on> tag will timeout
  • @error-message - a message that is shown if the stream emits an error
  • @timeout-message - a message that is shown if the stream times out
  • FUTURE: @placeholder-message - a message that is shown while the <on> tag is streaming
  • @empty-message - a message that is shown if the stream ends without emitting data
  • @finish-message - a message that is shown when/if the stream completes successfully

Also, we'll provide <on-placeholder>, <on-error>, <on-timeout>, <on-empty>, and <on-finish> as alternatives *-message attributes.

@mlrawlings
Copy link
Member Author

mlrawlings commented Jun 21, 2016

Example time:

Let's say we want to create a list that has a few users in it and a button to load more users. Those users come from some service call. With <await> we'd do something like this:

<await(users from data.usersProvider) client-reorder>
    <await-placeholder>
        <card>
            <loading-indicator/>
        </card>
    </await-placeholder>
    <for(user in users)>
        <card>
            <img src=user.avatar />
            <info>
                <name>${user.name}</name>
                <handle>${user.handle}</handle>
            </info>
        </card>
    </for>
    <button>load more</button>
</await>

In this case, the placeholder card would spin until the data gets back from the service call, then we render the user cards and load more button and replace the placeholder with these.

With <on> and a stream, we could do this instead:

<on(user in data.usersStream) client-reorder>
    <on-placeholder>
        <card>
            <loading-indicator/>
        </card>
    </on-placeholder>
    <card>
        <img src=user.avatar />
        <info>
            <name>${user.name}</name>
            <handle>${user.handle}</handle>
        </info>
    </card>
    <on-finish>
        <button>load more</button>
    </on-finish>
</on>

In this case, the placeholder would spin until all user cards have loaded. As each user is emitted by the stream, a user card would be rendered and inserted right before the loading card. Once all user cards were loaded, the load more button would be displayed and the placeholder removed.

Here's a mockup of the difference.

@mlrawlings
Copy link
Member Author

More thoughts:

It might be useful be able to see what iteration you're on like the <for> tag's status-var.

  • Each time through the status var would contain the current iteration: 0, 1, 2, 3 ...
  • On the end event the status var would contain the number of iterations, similar to the way things work in a native for loop:
for(var i = 0; i < array.length; i++) {
    console.log(array[i]);    
}
assert(i === array.length);

This could be used to:

  • show a different timeout-message or error-message base on whether any data had already been rendered:
<on(user in data.usersStream | status-var=loop)>
    <on-timeout>
        <if(loop.getIndex() > 0)>
            We timed out, but at least we got some data!
        </if>
        <else>
            We timed out and got nothing...
        </else>
    </on-timeout>
    ${user.name}
</on>
  • In the same way, it removes the need for empty-message

@GenaANTG
Copy link

GenaANTG commented Jul 2, 2016

Good idea! I need it.

@scttdavs
Copy link
Contributor

This sounds great! We would be able to use this right away. We're basically doing this manually in a renderer.js file ourselves now, but this is much cleaner and doesn't require us to expose all the out.beginAsync stuff to create our own stream.

Are there any strong feelings in making this a separate tag? Would it make sense to just have <await> accept a stream in addition to promises and callbacks? Or does that have mostly to do with the terminology of await in ES2017?

@mlrawlings
Copy link
Member Author

mlrawlings commented Nov 22, 2016

@scttdavs The main difference, is in passing a stream to <await> you would need to buffer the stream until it completes, where as <on>'s body would be called every time a data event was emitted from the stream and could flush out multiple times.

I actually just threw something together this afternoon because another team at eBay was asking about this. It doesn't have any of the timeout/placeholder/buffering/flushing/etc. in place, but it's a good starting point.

I probably won't have time to revisit this for a little while, so if you're interested in working on this, that would be awesome!

/cc @RajaRamu (on the aforementioned team at eBay)

@mlrawlings mlrawlings added this to the 4.0 milestone Nov 22, 2016
@Hesulan
Copy link
Contributor

Hesulan commented Nov 24, 2016

If there's enough interest, it would be fairly simple for <await> to also accept streams - though as Michael pointed out, it would need to buffer the stream until it was finished, so I'm not sure how useful that would be.

Alternatively, most streams can simply be converted to Promises with something like

new Promise(function(resolve, reject) {
    var buffer = [];
    stream.on('error', reject);
    stream.on('data', function(data) {
        buffer.push(data);
    });
    stream.on('end', function() {
        var result = Buffer.concat(buffer).toString();
        resolve(result);
    });
});

Hopefully this weekend I'll get the chance to play with the <on> tag a bit, looks awesome!

@patrick-steele-idem
Copy link
Contributor

I think there's enough differences between handling a Promise vs a ReadableStream/EventEmitter that it makes sense to keep the tags separate and I think it will make things less confusing and easier to document. I could be wrong though and I might be convinced that <await> could be extended to support both a Promise that gets flushed once when resolved versus a ReadableStream/EventEmitter that will flush for every event. We still need to finalize this proposal before doing any work so if you have any thoughts on usage please share.

@Hesulan
Copy link
Contributor

Hesulan commented Nov 24, 2016

@patrick-steele-idem If <await> was to handle multiple-flush streams / event emitters (which I don't believe it should), I would prefer something similar to "deferred.progress()"; perhaps a separate await-specific nested tag, like <await-on-data>.

Though here's another thought that just popped to mind: What about something like that for the <on> tag? For example...

<ul on(streamOrEventEmitter)>
    <li on-data(result)><!-- maps to ".on('data', ...)" -->
        ${result}
    </li>
</ul>

...as an alternate syntax to allow binding to events other than just 'data'?

@Hesulan
Copy link
Contributor

Hesulan commented Nov 24, 2016

On second thought, something like this might be better instead:

<ul>
    <li on(var1, var2 from 'eventName' of emitter)>
        ${var1 + ', ' + var2}
    </li>
</ul>

('from' and 'of' are just what came off the top of my head, it may need re-wording)

@scttdavs
Copy link
Contributor

I kind of like that implementation too. One concern I would have is that there's no clear way to use <on-finish> from Michael's example (or any of the other sub-tags).

I think <on-finish>, in particular, is important because given his example linked above, the one of a load more button below the users. What if it is not known whether there are more users to load until data from the stream comes in? In which case you may need a load more button or may not. So being able to render it after the steam ends when that information is available would solve that.

@Hesulan
Copy link
Contributor

Hesulan commented Nov 28, 2016

(Just noticed the original post had an example of using a generic EventEmitter which accomplishes basically the same thing as my previous post, not sure how I missed that a few days ago.)

Hmm... Rather than starting with standard streams and adding different syntaxes to support non-standard use cases, what if we looked at this the other way around? We could start by exposing the generic behavior:

<ul>
    <li on(user from 'data' in data.usersStream) repeat=true>
        ${user.name}
    </li>
    <li on('end' in data.usersStream)>
        <button>load more</button>
    </li>
</ul>

Then add convenience syntaxes for the most common uses:

<ul>
    <li on(user from data.usersStream)>
        ${user.name}
        <on-finish>
            <li><button>load more</button></li>
        </on-finish>
    </li>
</ul>

Note that <on(user from data.usersStream)> and <on-finish> are just syntactic sugar for <on(user from 'data' in data.usersStream) repeat=true> and <on('end' in data.usersStream)>, respectively.

@patrick-steele-idem
Copy link
Contributor

I'll throw out another proposal:

<ul>
    <await(data.usersStream)>
        <on('data' as user)>
            <li>${user.name}</li>  
        </on>
    </await>
</ul>

@RajaRamu
Copy link

RajaRamu commented Dec 1, 2016

@patrick-steele-idem - this sounds good to me. Looks lot cleaner!

@patrick-steele-idem
Copy link
Contributor

patrick-steele-idem commented Dec 1, 2016

@mlrawlings @philidem @tindli @austinkelleher @Hesulan @scttdavs should we go forward with using <await> with nested <on()> tags (see above)?

@RajaRamu
Copy link

RajaRamu commented Dec 2, 2016

@patrick-steele-idem @mlrawlings - will accept event emitter going forward. right now, its not supporting event emitter. i think its essential to support that with respect to streaming

@Hesulan
Copy link
Contributor

Hesulan commented Dec 2, 2016

I can't think of any reason not to. Just one question: If the object has both .on() and .then() methods, should it be treated as an emitter or a thenable? (I'm leaning toward emitter, but that might be considered a breaking change...)

Also, a few more ideas:

  • <await(data.userStream until 'finished' or 'cancelled')>
  • <on(user)> (defaults to 'data' event)
  • <once(...)>

@austinkelleher
Copy link
Member

@patrick-steele-idem - I like that syntax a lot.

@scttdavs
Copy link
Contributor

scttdavs commented Dec 3, 2016

@patrick-steele-idem I like the new proposal, especially reusing with <await> since the use cases seem so similar. So given that example and the new nested tag feature (which is done now?), would it look like:

<ul>
    <await(data.usersStream)>
        <@on('data' as user)>
            <li>${user.name}</li>  
        </@on>
        // how is content here handled?
    </await>
</ul>

I also had a question in the comment above^

@mlrawlings
Copy link
Member Author

mlrawlings commented Dec 5, 2016

There's the potential argument against <@on> because it would need to be transformed:

<@on('data' as user)> would get transformed to <@on(user) event='data'> which would generate code like:

{
    on:[{
        event:'data',
        renderBody:function(out, user) {
            out.w('<li>'+escapeXML(user.name)+'</li>';
        }
    }]
}

@scttdavs The rules around content outside @ tags is a little weird... Anything outside an @ tag will come through as input.renderBody, except if there are dynamic @ tags in which case renderBody is run and the @ tags register themselves on the parent and in that case, no content output is allowed in the renderBody.

In the case of the <await>/<@on> combination, renderBody would not be called. We should probably thrown an error in that case.

@mlrawlings mlrawlings modified the milestone: 4.0 Mar 13, 2017
@mauricionr
Copy link

@mlrawlings @austinkelleher @patrick-steele-idem

any tips and tricks for applying infinite scrolling with marko?

@DylanPiercey
Copy link
Contributor

DylanPiercey commented Apr 16, 2024

FYI this can be done in userland. Although maybe it eventually becomes a core tag:

// usage
<for-await|item| of=asyncIterable>
  
</for-await>

// impl
$ const iterable = input.of[Symbol.asyncIterator]();

<macro name="next">
  <await(iterable.next())>
    <@then|item|>
      <${input.renderBody}(item.value)/>
      <if(!item.done)>
        <next/>
      </if>
    </>
  </>
</macro>

<next/>

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

No branches or pull requests

9 participants