Maybe use a discoverable script tag instead of special tag?

That's an option. If the performance of a DOM scan is acceptable, then these datasets can also be picked up at that time.

The layout=nodisplay is particularly annoying here.

We just need to remove it.

Is this an assignment?

Yes, it effectively means selected = ampEventData;.

Missing opening tag or extra closing.

Requiring these for non custom elements may be a reasonable requirement we should discuss.

Missing opening tag or extra closing.

Oops, removed.

Requiring these for non custom elements may be a reasonable requirement we should discuss.

True, that would avoid the need for a DOM scan but is a bit annoying. Using a single element for multiple bindings would be less verbose, e.g.:

<p>
  <amp-bind 
      bind-text="Some text {someVar}"
      bind-class="{someClass: someExpr}">
  </amp-bind>
</p>

We just need to remove it.

In this notation, we should try to avoid amp-bind-attribute and others. Instead, there should be a single <amp-bind> with the affect described as attributes.

Agreed. My reply to Malte above has a sample:

<p>
  <amp-bind 
      bind-text="Some text {someVar}"
      bind-class="{someClass: someExpr}">
  </amp-bind>
</p>

Cool. This is definitely better, imho.

What does this "bind" do exactly? Changes the selected slide of a carousel? How? Please describe possible values.

What does this "bind" do exactly? Changes the selected slide of a carousel?

Yes, it updates the carousel's currently displayed slide when the local state var selected changes.

What's the local var selected? Does it belong to any object? Or is it created on request?

It's created on request.

What is amp-bind-event? Why is it needed? How is it different from just amp-bind above? Generally, to me, presence of events immediately communicates two-way binding which we are here trying to avoid.

amp-bind-event is for binding local state changes to user actions. My proposal uses AMP's existing on="" in lieu of amp-bind-event.

To clarify, this file uses syntax based off of Ali's hackathon PR. The other file, "bind-shopping.proposed.amp.html", contains my proposed syntax. I included both in this PR for comparison purposes. Sorry for any confusion.

I'd like to see if we can express everything solely as a state change events. E.g. instead of binding to slidechange event, we'd bind to carousel1.selectedSlide state. This makes uni-directionality explicit. And in this particular case, it may completely get rid of the need of variable binding.

Hmm, good point. That markup could look like this:

<amp-carousel slide=0 id="carousel-1">
</amp-carousel>

<amp-img id="img-1"
    on="tap:#carousel-1.setSlide(0)"
    bind-attr="{selected: #carousel-1.slide === 0}">
</amp-img>

This would save a few LOC and an indirection. However, now the "current selection" state lives in #carousel-1 and all references of selected will need to be replaced by #carousel-1.slide. If we instead keep selected var, then the state lives in two places. So I think this may be locally convenient but globally inconvenient.

And then imagine a third element being bound to #img-1, and then #carousel-1 can be bound to it creating a cycle, etc. If developers aren't careful, this can create a sort of "bind spaghetti" where application state is difficult to keep track of.

My understanding is that frameworks like Flux, Redux, etc. advocate making a similar tradeoff ("unidirectional data flow") to avoid this class of problem.

Redux says:

UI state is also increasing in complexity, as we need to manage the active route, the selected tab, whether to show a spinner or not, should pagination controls be displayed, and so on.

Managing this ever-changing state is hard. If a model can update another model, then a view can update a model, which updates another model, and this, in turn, might cause another view to update. At some point, you no longer understand what happens in your app as you have lost control over the when, why, and how of its state.

So I think forcing all bindings to update local state first:

• Avoids "bind spaghetti", cycles, etc. by developers
• Makes applying performance constraints to eval/update easier and more predictable
• Is familiar to React developers
• Is more straightforward to implement

It's not a perfect comparison since amp-bind is not meant to manage a web app's entire state, unlike these frameworks. At least, I think not having "bind-to-element-state" may be sufficient for V1. I'd defer to your judgment and experience though. :)

Why is there a choice of carousel here vs just a amp-img with src binding?

It's one of the use cases listed by Eric and is quite common in online clothing sites, e.g. Uniqlo (view on mobile).

What does this button do?

Selecting a size un-expands the amp-accordion's section, and this button is displayed to allow modification. This is modeled off of one of Eric's use cases (Google Store's product pages for phones). Please check out the video demo.

Do we really need an event for what is an intrinsic property of a carousel - "selected slide"?

For V1, I think it's clearer to force developers to use local state as an intermediary (it's also more reminiscent of React/Flux). Binding element A's state directly to element B's state feels more like two-way data binding.

What's the difference between thumbnails and carousel above?

The carousel displays the large, hero-style image. See the video demo.

Cool, this looks like a great start. Some overall notes I have:

We need to make sure this is easy enough to validate. E.g. we can’t allow setting onclick this way. We can leave it to runtime, of course. But validator is often handy.

takes a string with expressions embedded in {} (string interpolation)

I think we should go for double-bracing: {{}}, which I think you are doing in most of examples.

Using custom attributes avoids "flash of unbound content".

Please clarify what this means.

Even if we go with a expanded form, we should still use string interpolation for all strings. I.e. avoid +.

This approach requires a one-time DOM scan to detect all bindings. This may have performance implications, but may be fine assuming that digest/evaluations are scheduled after initial layout

A. One-time scan is not enough. We will also have to pick up deletes and other mutations via amp-live-list and other components.

B. This scan is potentially very expensive, especially with short-cut forms such as bind-src.

That being said, I agree that we should experiment with attribute-based binding. However, I'd like to stress that from the model point of view, these two solutions are equivalent. Thus we first should work on reducing the algebra. I believe we can drastically reduce number of forms and expression syntax.

Thanks for the review!

We need to make sure this is easy enough to validate. E.g. we can’t allow setting onclick this way.

Agree with validation. Not sure what you mean by onclick, did you mean the on attribute? Why is that a problem?

I think we should go for double-bracing: {{}}

May I ask why? Angular uses double-bracing for string interp. while React doesn't.

Please clarify what this means.

"Flash of unbound content" just means that binding to default attributes like src may cause the browser to render/load unparsed expressions. Angular describes this in its ngBind doc.

A. One-time scan is not enough. We will also have to pick up deletes and other mutations via amp-live-list and other components.

B. This scan is potentially very expensive, especially with short-cut forms such as bind-src.

Good point! Though for amp-live-list, we'll only have to scan the updated children rather than rescanning the entire DOM. Assuming a modest number of updates, do you think this is still a problem?

More broadly though, we certainly may need apply constraints for performance, e.g. re-evaluation frequency, cap on the absolute number of bindings, etc.

I'd like to stress that from the model point of view, these two solutions are equivalent. Thus we first should work on reducing the algebra. I believe we can drastically reduce number of forms and expression syntax.

Agreed. :)

Not sure what you mean by onclick, did you mean the on attribute? Why is that a problem?

The onclick is an example of an AMP attribute that we do not allow for security reasons. Since we don't allow it in our static validator, we should also not allow it from binding resolutions.

May I ask why? Angular uses double-bracing for string interp. while React doesn't.

I can be convinced that a single brace is enough. The only reservation I have is that if the probability of seeing a { in the natural content is p, then it will be p^2 for {{. :)

"Flash of unbound content" just means that binding to default attributes like src may cause the browser to render/load unparsed expressions. Angular describes this in its ngBind doc.

We decided to explicitly disallow first-load rebinding. Right?

Good point! Though for amp-live-list, we'll only have to scan the updated children rather than rescanning the entire DOM. Assuming a modest number of updates, do you think this is still a problem?

Not really a problem. But important to acknowledge that we'd create a problem that we wouldn't have with a custom element. It's a tradeoff, so we need to make sure we get our +/- right.

The onclick is an example of an AMP attribute that we do not allow for security reasons. Since we don't allow it in our static validator, we should also not allow it from binding resolutions.

Got it. We'll only allow/parse expressions in bind-attr, bind-class and bind-text and enforce it in the validator.

I can be convinced that a single brace is enough. The only reservation I have is that if the probability of seeing a { in the natural content is p, then it will be p^2 for {{. :)

True. React may not be a great example since markup and code are mixed together which reduces unintended collision -- I'll do some digging here.

We decided to explicitly disallow first-load rebinding. Right?

Yes, but I think this is nice because:

• Allows developers to easily set the initial value of an attribute
• We only need to check a few bind- attributes (vs. all attributes) for bindings

Not really a problem. But important to acknowledge that we'd create a problem that we wouldn't have with a custom element. It's a tradeoff, so we need to make sure we get our +/- right.

Absolutely. Before we go with DOM scanning, I'll performance test "worst-case" pages on slow hardware and make sure performance is acceptable. You may be right that using a single <amp-bind> for non-AMP elements is the better solution.

are we going to make interpolation marker configurable? I know multiple CMS's that have problem with not being able to change it

are we going to make interpolation marker configurable?

Good idea. We should make it configurable.

I'd really like to have an id here. It might seem a small thing, but I think it's a very important aspect and the one I failed to fully communicated before.

Why is requiring an id important here?

At this time, I'd like to see if we really need a "default dataset" kind of case. If we always use ID - then we can reference state of any object. I.e.

bind="dataset1.item[0].sizes"

can coexist naturally with

bind="carousel1.selectedIndex"

and

bind="checkbox1.selected"

Oh I see. Hmm, I think having a "default dataset" is a bit simpler and more convenient for most use cases:

• It's nice to be able to set local vars without an explicit scope (e.g. in React)
• Multiple <amp-bind-dataset> in a single page is probably uncommon and scoping is still possible manually
• May be more convenient to deal with XHR data

Requiring scoping datasets by id is probably cleaner for more complicated pages, but the e-commerce product pages and other examples on Eric's doc are pretty simple.

It's one of the use cases listed by Eric and is quite common in online clothing sites, e.g. Uniqlo (view on mobile).

Yes, it effectively means selected = ampEventData;.

amp-bind-event is for binding local state changes to user actions. My proposal uses AMP's existing on="" in lieu of amp-bind-event.

To clarify, this file uses syntax based off of Ali's hackathon PR. The other file, "bind-shopping.proposed.amp.html", contains my proposed syntax. I included both in this PR for comparison purposes. Sorry for any confusion.

Agreed. My reply to Malte above has a sample:

<p>
  <amp-bind 
      bind-text="Some text {someVar}"
      bind-class="{someClass: someExpr}">
  </amp-bind>
</p>

What does this "bind" do exactly? Changes the selected slide of a carousel?

Yes, it updates the carousel's currently displayed slide when the local state var selected changes.

Yes. The document has a single scope for local state, which is initialized with <amp-bind-dataset>. The assignment is done on that local state.

Selecting a size un-expands the amp-accordion's section, and this button is displayed to allow modification. This is modeled off of one of Eric's use cases (Google Store's product pages for phones). Please check out the video demo.

The carousel displays the large, hero-style image. See the video demo.

For V1, I think it's clearer to force developers to use local state as an intermediary (it's also more reminiscent of React/Flux). Binding element A's state directly to element B's state feels more like two-way data binding.

My proposed syntax builds on the existing on="tap:<expr>" system. Please check out bind-shopping.proposed.amp.html.

It's created on request.

That's an option. If the performance of a DOM scan is acceptable, then these datasets can also be picked up at that time.

Oh I see. Hmm, I think having a "default dataset" is a bit simpler and more convenient for most use cases:

• It's nice to be able to set local vars without an explicit scope (e.g. in React)
• Multiple <amp-bind-dataset> in a single page is probably uncommon and scoping is still possible manually
• May be more convenient to deal with XHR data

Requiring scoping datasets by id is probably cleaner for more complicated pages, but the e-commerce product pages and other examples on Eric's doc are pretty simple.

The onclick is an example of an AMP attribute that we do not allow for security reasons. Since we don't allow it in our static validator, we should also not allow it from binding resolutions.

Got it. We'll only allow/parse expressions in bind-attr, bind-class and bind-text and enforce it in the validator.

I can be convinced that a single brace is enough. The only reservation I have is that if the probability of seeing a { in the natural content is p, then it will be p^2 for {{. :)

True. React may not be a great example since markup and code are mixed together which reduces unintended collision -- I'll do some digging here.

We decided to explicitly disallow first-load rebinding. Right?

Yes, but I think this is nice because:

• Allows developers to easily set the initial value of an attribute
• We only need to check a few bind- attributes (vs. all attributes) for bindings

Not really a problem. But important to acknowledge that we'd create a problem that we wouldn't have with a custom element. It's a tradeoff, so we need to make sure we get our +/- right.

Absolutely. Before we go with DOM scanning, I'll performance test "worst-case" pages on slow hardware and make sure performance is acceptable. You may be right that using a single <amp-bind> for non-AMP elements is the better solution.

Interesting. Agree that templating could reduce markup -- do you have more info on <amp-select-one>? Couldn't find it in my search.

Hmm, good point. That markup could look like this:

<amp-carousel slide=0 id="carousel-1">
</amp-carousel>

<amp-img id="img-1"
    on="tap:#carousel-1.setSlide(0)"
    bind-attr="{selected: #carousel-1.slide === 0}">
</amp-img>

This would save a few LOC and an indirection. However, now the "current selection" state lives in #carousel-1 and all references of selected will need to be replaced by #carousel-1.slide. If we instead keep selected var, then the state lives in two places. So I think this may be locally convenient but globally inconvenient.

And then imagine a third element being bound to #img-1, and then #carousel-1 can be bound to it creating a cycle, etc. If developers aren't careful, this can create a sort of "bind spaghetti" where application state is difficult to keep track of.

My understanding is that frameworks like Flux, Redux, etc. advocate making a similar tradeoff ("unidirectional data flow") to avoid this class of problem.

Redux says:

UI state is also increasing in complexity, as we need to manage the active route, the selected tab, whether to show a spinner or not, should pagination controls be displayed, and so on.

Managing this ever-changing state is hard. If a model can update another model, then a view can update a model, which updates another model, and this, in turn, might cause another view to update. At some point, you no longer understand what happens in your app as you have lost control over the when, why, and how of its state.

So I think forcing all bindings to update local state first:

• Avoids "bind spaghetti", cycles, etc. by developers
• Makes applying performance constraints to eval/update easier and more predictable
• Is familiar to React developers
• Is more straightforward to implement

It's not a perfect comparison since amp-bind is not meant to manage a web app's entire state, unlike these frameworks. At least, I think not having "bind-to-element-state" may be sufficient for V1. I'd defer to your judgment and experience though. :)

I think we will need a primitive for URL construction that can be discovered by cache rewriting.

In any case, we need to know that src is a dangerous URL field.