Giter Site home page Giter Site logo

Comments (25)

robdodson avatar robdodson commented on July 23, 2024 1

šŸ‘ great work @JanMiksovsky !

from gold-standard.

treshugart avatar treshugart commented on July 23, 2024 1

Sorry I never got a chance to weigh in, @robdodson, but thanks for cc'ing me. Most everything you've put down I'd echo, though, due to the holidays I haven't had a chance to give it the attention I probably should have. Great work @JanMiksovsky :)

from gold-standard.

robdodson avatar robdodson commented on July 23, 2024

I heard today that Polymer 0.8 might do this for you, @sjmiles is that true(ish)?

I realize this issue extends beyond Polymer because you could be creating a vanilla Web Component, but wanted to point that out if it is true

from gold-standard.

chiefcll avatar chiefcll commented on July 23, 2024

I think this can be closed out thanks to @robdodson article:
http://robdodson.me/interoperable-custom-elements/

I'm on board for accepting those guidelines.

from gold-standard.

JanMiksovsky avatar JanMiksovsky commented on July 23, 2024

I'm not ready to accept Rob's suggestion that components only raise events for user interactions. @robdodson and I will discuss that, and determine how we want to update the checklist.

from gold-standard.

JanMiksovsky avatar JanMiksovsky commented on July 23, 2024

To kick off the discussion, I wanted to capture some questions I have on this topic.

Here's the relevant part of Rob's article:

When I showed this pattern off recently someone asked if it would be better to dispatch the events inside of the property setters, for example:

set checked(val) {  
  this._checked = val;
  /* do any other updates we need */
  this.dispatchEvent(new CustomEvent(ā€˜check-changedā€™, {
      detail: checked: val
  }));
}

The danger with doing this is you may end up creating an infinite loop if thereā€™s an external listener for the event which in turn updates the property. Looking again to native elements, it seems that their event dispatching only takes place when some external force is acting upon the element, e.g. someone clicked, someone typed, something loaded, etc. Hence, we donā€™t dispatch check-changed when the public property is set, but instead only when someone clicks on the checkbox.

First, I want to observe that Rob's writing about properties, while this issue is about attributes. So the first question is: do we want to treat those differently? That is, do we want component authors to raise events when an attribute changes, or when a property changes, or both? Those options are related, because most authors will have an attributedChangedCallback set the corresponding property.

A second question is: Does the standard HTML behavior make sense globally for an essentially infinite number of custom elements? A checkbox doesn't raise its change event when its checked property changes. The premise of the Gold Standard is to emulate the standard HTML elements, so a priori, we assume we want custom elements to behave the same way. I think that's a good starting point. But if it makes life difficult, I'm also open to reconsidering that stance. The world only has ~100 standard HTML elements, only a tiny number of which make up the bulk of interactive behavior. At that scale, documentation of special cases or behaviors is workable. But there will be zillions of custom elements, and itā€™s possible that what makes sense for that scale will be different.

A third, related question is: Is the standard HTML behavior really obvious? I actually hadnā€™t known (or had forgotten) that a checkbox doesnā€™t raise a change event when its checked property changes. That behavior was surprising to me. Itā€™s probably surprising to other devs. And, from Robā€™s wording, ā€œIt seems that their event dispatching only takes placeā€¦ā€, I infer that Rob didnā€™t know with absolute certainty how a really basic feature of a really basic component worked, and had to resort to poking at a checkbox to see what it did. Robā€™s a super-smart guy, and letā€™s conservatively put him in the 99th percentile of knowledgable web developers. Iā€™ll put myself in the 90th percentile. If weā€™re surprised or not sure when events fire in these cases, then I have to assume many other people will be even more confused. It seems much simpler to be able to say, ā€œWhenever property foo changes, it raises a foo-changed event."

Fourth question: How would data binding systems (Polymer, Ember, etc.) work without change events for properties? From Polymerā€™s docs on change notification events, it seems that Polymer relies on change events being fired when there is a "change to a notifying property.ā€ Without that, how can Polymer know that an underlying property value has changed and should be propagated along any existing bindings? Iā€™m not familiar with Ember and similar frameworks, but assume they need such events too.

Fifth question: Iā€™m not completely certain of this, but given the above issue, Iā€™m concerned that people composing complex UI components from multiple, smaller components will have to carefully keep track of which things are happening in direct response to user interactions and which are happening because of changes imposed by the outer application. This is similar to the data binding question above; here the difference is that a component author might not be using a framework with data binding, but rather gluing the smaller constituent components together by hand. Iā€™m worried such glue code will become more complex if the component author has to track the source of a property change.

Sixth question: How can component behavior be properly factored if there is a direct relationship between user interactions and property effects? Case in point: the Elix RFC for a single-selection mixin proposes a mixin that manages abstract single-selection semantics for components like lists. Itā€™s analogous to Polymerā€™s iron-selectable-behavior, but by design the mixin is completely abstract, and has no connection to the DOM. We think thatā€™s a good thing. The mixin presumes that changing a property like an objectā€™s selectedItem should raise a selected-item-changed event. That is very easy to arrange for in the setter for selectedItem, and results in a simple API. But if the component should only raise change events in response to user interaction, the mixin needs some more complex API in which the base component needs to let the mixin know whether a change to selectedItem is happening as a result of user interaction (and should raise an event) or happening for some other reason. That seems quite complex.

Seventh question: How should components handle situations in which they update multiple properties in response to a single user event? Again using that single-selection mixin example, that proposal supports modifications of a componentā€™s selection in two ways: 1) by item index or, 2) by an object reference to the item. These two properties are linked: if you change the selectedIndex property, the selectedItem property updates too, and vice versa. This compounds the issue raised in the above point.

Eighth question: Should the prevention of the infinite loop be the dominant concern here? Rob points out, "The danger with doing this is you may end up creating an infinite loop if thereā€™s an external listener for the event which in turn updates the property.ā€ The suggestion to not raise change events for property updates attempts to avoid the possibility of such an infinite loop. But that places a significant burden on the component author. Another solution is to push the responsibility on the component user: i.e., a change event handler shouldnā€™t update the indicated property on the original component.

from gold-standard.

robdodson avatar robdodson commented on July 23, 2024

I spoke with some Blink engineers about this. Let me try to answer the above points in order.

So the first question is: do we want to treat those differently? That is, do we want component authors to raise events when an attribute changes, or when a property changes, or both? Those options are related, because most authors will have an attributedChangedCallback set the corresponding property.

As a general rule of thumb, I don't think we want every property/attribute to dispatch an event. To quote one of the Blink engineers: "In the platform today most 'property changed' events don't fire if you manually set the property because it's assumed that you were the one setting it." It would also be quite spammy and possibly expensive if every property change dispatched an event.

Having said that, there may be exceptions because the platform is not consistent here. Apparently setting the src property of an img dispatches a load event.

Does the standard HTML behavior make sense globally for an essentially infinite number of custom elements?

I think it's a good model to follow and it's what frameworks and libraries are already written to understand.

A third, related question is: Is the standard HTML behavior really obvious?

I guess for me it's non-obvious only because prior to Custom Elements I didn't spend much time building components to exactly model what the DOM does today. But I think it's a teachable pattern. Again, saying every property should dispatch an event will probably get very expensive/spammy.

Fourth question: How would data binding systems (Polymer, Ember, etc.) work without change events for properties?

The same way it listens to native checkbox today. Polymer's notify stuff is primarily for two-way data binding but two-way bindings are an opinionated, framework-specific pattern. It's not something the platform natively supports and it's not even something that all frameworks universally support or favor.

Fifth question: Iā€™m not completely certain of this, but given the above issue, Iā€™m concerned that people composing complex UI components from multiple, smaller components will have to carefully keep track of which things are happening in direct response to user interactions and which are happening because of changes imposed by the outer application.

This is how native HTML works today and I don't think we have major issues there. Each element I'm composing is essentially a black box to me, so I can ignore why it's dispatching its change event and just listen for it.

Sixth question: How can component behavior be properly factored if there is a direct relationship between user interactions and property effects?

I think this becomes less of an issue if we remove the mixin from the equation. Personally I question the value of behavior mixins. I realize not having mixins leads to code duplication in elements but I've seen first hand how elaborate and confusing mixins have made some Polymer Elements. I'm not a big fan but that's just my personal opinion :)

Does the mixin have to use an event dispatch? Could it use a callback instead? Maybe override the setter and whenever it gets called, at the very end do callback(this.selectedItem).

Seventh question: How should components handle situations in which they update multiple properties in response to a single user event?

Does removing the mixin make this easier to reason about?

Eighth question: Should the prevention of the infinite loop be the dominant concern here?

It seems fair to want the frameworks out there watch out for this. But I think another reason to avoid having every property fire an event would be to avoid spamminess.

I'm curious what @treshugart thinks about all this :D

from gold-standard.

JanMiksovsky avatar JanMiksovsky commented on July 23, 2024

@robdodson Thanks for the thoughtful response! It's really helpful.

Since I donā€™t want opinions about mixins to color this discussion, letā€™s set that topic aside. At some point, Iā€™d like to have the opportunity to show you why we think theyā€™re actually a suitable pattern for complex components, and how we can avoid some of the issues Polymer ran into with behaviors ā€” but that can wait.

Other pointsā€¦

To quote one of the Blink engineers: "In the platform today most 'property changed' events don't fire if you manually set the property because it's assumed that you were the one setting it." It would also be quite spammy and possibly expensive if every property change dispatched an event.

Performance is a compelling argument. Iā€™m willing to see if we can rewrite our components to be able to only generate change events for UI interactions. Iā€™m concerned weā€™ll run into issues, but thereā€™s no point in my arguing about that until Iā€™ve tried and can speak directly to experience.

Does the standard HTML behavior make sense globally for an essentially infinite number of custom elements?

I think it's a good model to follow and it's what frameworks and libraries are already written to understand.

I remain concerned that whatā€™s worked for a relatively tiny set of extremely well-known, simple elements may not scale to a much larger number of components which are possibly much more complex. In particular, Iā€™m worried that it will often be desirable for components to expose two or more properties with implicit relationships. As just one example, letā€™s look at google-map:

  • google-map has properties for latitude, longitude, fitToMarkers, and zoom. Those properties have implicit relationships: setting one of the properties may affect another. Those relationships may not be apparent without reading the documentation and code.
  • The documentation for fitToMarkers, for example, suggests that setting that can affect zoom, but it appears to also affect latitude and longitude as well.
  • If more of the underlying Map object properties were exposed ā€” say, map bounds ā€” the number of interrelated properties would increase.
  • Now consider that a componentā€™s children (including distributed children) could affect properties. If I add another google-map-marker child, and have fitToMarkers set, what happens then?

The point is that the implicit relationships make it hard for the dev directing the action to know what can effect what. Suppose Iā€™m gluing a google-map to both components that can control the map, and to components that reflect map properties like lat/long. Without property change events, I donā€™t know whether setting fitToMarkers will affect zoom. I have to do something like: if the map-controlling components want to change fitToMarkers, I remember to check to see whether zoom changed, and then propagate the change to update the components reflecting map properties.

With property change events, I would have just told the map property-displaying components to listen for change events in the properties they render. When my map-controlling components poke at the map, properties update, change events are raised, and new properties are rendered.

Here the sheer number of components matters. The small number of standard elements changes at a glacial pace, and thereā€™s ample time to learn how those elements work. If, however, we're dropping new components into our projects on a daily basis, it may be very hard to know which properties will have implicit affects on other properties.

A third, related question is: Is the standard HTML behavior really obvious?

I guess for me it's non-obvious only because prior to Custom Elements I didn't spend much time building components to exactly model what the DOM does today. But I think it's a teachable pattern. Again, saying every property should dispatch an event will probably get very expensive/spammy.

I agree that this could be taught. Iā€™d be happier with a simpler model (change a property by any means => raise event), but given the performance concerns you raise, I donā€™t see much of a choice.

Fourth question: How would data binding systems (Polymer, Ember, etc.) work without change events for properties?

The same way it listens to native checkbox today. Polymer's notify stuff is primarily for two-way data binding but two-way bindings are an opinionated, framework-specific pattern. It's not something the platform natively supports and it's not even something that all frameworks universally support or favor.

I just wanted to point out that the question isnā€™t necessarily about two-day data binding. In the map example above, the bindings (managed through a data-binding layer, or by hand) could be one way.

from gold-standard.

robdodson avatar robdodson commented on July 23, 2024

ok so to summarize, if changing propA of an element causes it to internally change the value of propB, how does the outside world know that propB just changed.

The options are:

  • Any property change dispatches an event, OR
  • Changing a property that also changes another property dispatches an event, OR
  • Developer has to know about these implicit relationships and check propB after they change propA

Does that seem about right? I'd like to get other folks opinions on this but want to try to distill the explanation down to something bite sized šŸ˜ø

from gold-standard.

JanMiksovsky avatar JanMiksovsky commented on July 23, 2024

Yes, I think you've captured that issue. Thanks for taking the time to boil this down.

from gold-standard.

robdodson avatar robdodson commented on July 23, 2024

@esprehn do you have an opinion on #1 (comment)

from gold-standard.

robdodson avatar robdodson commented on July 23, 2024

I'm going to quote @kevinpschaaf from the Polymer team here just to add some opinions.

If we weren't bound by the [Polymer] 1.x behavior, I would likely push to codify the following as a best practice: never fire property-changed events as a result of an outside actor changing the property (aka downward data flow); only fire them as a result of an internal change not initiated from outside actors (e.g. UI interaction, async tasks, network/service results, etc.). Reasoning is similar to topics discussed in [this] thread: the potential for i-loop is high (we only avoid this in 1.0 through strict equality dirty-checking, which is restrictive), it can lead to change processing-reentry which is difficult to reason about, notifying a property set from the host back to the host is superfluous and perf-negative, and this is generally the behavior of platform events

from gold-standard.

JanMiksovsky avatar JanMiksovsky commented on July 23, 2024

@robdodson and @kevinpschaaf: Thanks for the insights. We'll do our best to make this work, and raise any issues we find.

For the Gold Standard, I really want to offer prescriptive advice when possible about how to achieve a particular result. If we were to codify this and incorporate Kevin's advice, I'd feel better if there were a pattern devs could follow to implement it.

In particular, I want to make sure that the pattern can cover the cases like our complex components, where a handler for a user interaction event may invoke many layers of code that ultimately need to set a property. In such cases, we're concerned that it will difficult for a component author to properly raise property change events.

One simple pattern we're trying out is:

  1. A component sets an internal flag at the beginning of any UI event handler or other internal events the host can't know about. At the end of the handler, the flag is cleared.
    const handlingUserInteractionSymbol = Symbol();

    this.addEventListener('click', event => {
      this[handlingUserInteractionSymbol] = true;
      this.foo = ... // Set a property, or call routines that do.
      this[handlingUserInteractionSymbol] = false;
    });
  1. Any property setter can check this flag to detect whether the work it's doing is in response to a UI/internal event, and should therefore raise a property change event.
    get foo() {
      ...
    }
    set foo(value) {
      ...
      if (this[handlingUserInteractionSymbol]) {
        this.dispatchEvent(new CustomEvent('foo-changed'));
      }
    }

See http://jsbin.com/nopafu/edit?html,console,output.

This pattern:

  • Is simple to explain.
  • Appears to implement the recommendation that property change events should only be raised in response to user interactions and other internal events the component's host can't know about.
  • Lets the property setter raise the corresponding event, colocating related code and ensuring the event is raised in response to all UI/internal events.
  • Does not require the UI/internal event handler to pass information down the call stack to indicate whether the current work is being done in response to an internal or external event.

Does this pattern meet the criteria?
Would you feel comfortable seeing this pattern in the Gold Standard treatment of this recommendation?

from gold-standard.

domenic avatar domenic commented on July 23, 2024

Maybe this is a good place to mention that users might expect canceling events to undo the change, like it does for input/change events in the DOM.

from gold-standard.

JanMiksovsky avatar JanMiksovsky commented on July 23, 2024

@robdodson I've taken a shot at adding a new guideline to the Gold Standard called Property Change Events. Per your recommendation, this indicates that property change events should only fire in response to internal component activity, and not when properties are set externally by the host. Can you please review?

@domenic I hadn't known that was possible. The MDN docs for change and input indicate the events are not cancelable. I also tried a spike and, per spec, don't see attempts to cancel the events succeeding. Can you update that example so I can see canceling of an input or change in practice?

from gold-standard.

domenic avatar domenic commented on July 23, 2024

@JanMiksovsky thanks for checking me; I was confused. It's the click event that's cancelable, and canceling it will undo the change on checkboxes and radio buttons. On reflection I think that's esoteric enough that it should be thought of more as a legacy mistake than as something expected. It's even named as such ("legacy-canceled-activation behavior").

from gold-standard.

JanMiksovsky avatar JanMiksovsky commented on July 23, 2024

@domenic Ah, okay. Thanks for the follow-up.

from gold-standard.

robdodson avatar robdodson commented on July 23, 2024

The recommendation in the wiki looks good to me.

from gold-standard.

JanMiksovsky avatar JanMiksovsky commented on July 23, 2024

Thanks! Since this has now been incorporated into the checklist, I'll close this issue out.

from gold-standard.

jouni avatar jouni commented on July 23, 2024

@robdodson, whatā€™s Polymerā€™s approach to this? Will the behaviour of notify: true be updated according to this recommendation at some point?

from gold-standard.

robdodson avatar robdodson commented on July 23, 2024

@jouni that's a good question. @kevinpschaaf do you know if Polymer 2 will change this behavior?

from gold-standard.

kevinpschaaf avatar kevinpschaaf commented on July 23, 2024

@jouni @robdodson Polymer 2.0 is intended to be mostly backward compatible w.r.t. the features in 1.x, so the notify:true behavior won't change. Regardless, its hard to envision how the notify: true behavior could be adapted to meet the recommendation since there's not enough information to know when the accessor is running whether the change was from the outside (e.g. from the host) or inside (e.g. as a result of internal event handling).

The recommendation really argues for not putting the event dispatch as a property-change side-effect (what notify: true does) and instead puts the onus on the author to properly dispatchEvent a change event when the element changes properties as a result of event handling. I think we would probably start there (manual), and then consider what sugar would help promote the pattern.

from gold-standard.

JanMiksovsky avatar JanMiksovsky commented on July 23, 2024

@kevinpschaaf FWIW, I was worried about adapting our components to support this recommendation. However, the simple raiseChangeEvents flag pattern described on the Property Change Events to discriminate between outside and inside property changes. This pattern appears to be working for us so far.

Polymer could consider whether it could define a similar flag that could be checked by its auto-generated setters for notify: true properties. There's a backward-compat problem: the raiseChangeEvents flag only works if its default value is false, and that's the opposite of the way notify: true works. But maybe there's a way to make that work.

from gold-standard.

kevinpschaaf avatar kevinpschaaf commented on July 23, 2024

@JanMiksovsky That's fair, but in either case given that you must do something to say whether an event should be fired, given the choice between twiddling some sugar or just doing the platform-idiomatic thing (call dispatchEvent where you want the event dispatched), at this point I'd probably just argue for less sugar. But yeah, that's an approach.

from gold-standard.

JanMiksovsky avatar JanMiksovsky commented on July 23, 2024

My experience has been that situating property change event dispatchers inside, e.g., UI event handlers is challenging. If you're designing an accessible component, for example, you'll end up with both mouse/touch and keyboard events. In complex components, it can also be hard to anticipate whether changing one property in a click handler will have second-order effects on other properties, so the click handler might have trouble figuring out just which property change events to raise. Finally, a UI event handler shouldn't raise a property change event if the property didn't actually change, and tracking that is a chore that a property setter may already be handling.

This experience has led to the raiseChangeEvents pattern, which divides responsibility for raising property changes events between the UI event handler (which just needs to signal that internally-triggered work is about to occur) and the property setter (which can look to see if a property has actually changed, and which can raise property change events in all circumstances).

YMMV, but it's working for us.

from gold-standard.

Related Issues (18)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    šŸ–– Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. šŸ“ŠšŸ“ˆšŸŽ‰

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ā¤ļø Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.