Idiomatic use of computed properties in Polymer 2

Question

I'm trying to wrap my head around how computed observables / properties in Polymer 2 work and should be used. I've created a repro analogous to my real code that zooms in on the thing I'd like to see reviewed: my use of computed properties in a Polymer 2 component.

Some things that might be helpful while reviewing:

• I've explicitly left out CSS and styling because I want to focus on getting the code right.
• We're targeting modern browsers, specifically for IE only Edge.

The main thing I'm worried about is hasSelectedPerson and its underlying "private" function. For one thing, it feels weird that I have to ask for a person as an argument, when it's also available as a property on the class.

<link rel="import" href="//polygit.org/polymer+:master/components/paper-button/paper-button.html">

<dom-module id="people-template">

  <template>
    <template is="dom-repeat" items="[[people]]" as="person">
      <paper-button raised on-click="_onSelectClick" item="[[person]]">
        [[person.name]]
      </paper-button>
    </template>
    <template is="dom-if" if="[[hasSelectedPerson]]">
      <p>Selected: [[selectedPerson.id]] [[selectedPerson.name]]</p>
    </template>
  </template>

  <script>
    class PeopleTemplate extends Polymer.Element {
      static get is() {
        return "people-template";
      }

      static get properties() {
        return {
          people: {
            type: Array,
            value: [
              { id: 1, name: "John Doe" },
              { id: 2, name: "Mary Havenstein" },
              { id: 3, name: "Janie van Staffelen" },
            ]
          },

          selectedPerson: Object,

          hasSelectedPerson: {
            type: Boolean,
            computed: "_hasSelectedPerson(selectedPerson)"
          }
        }
      }

      _onSelectClick(event) {
        this.set("selectedPerson", event.composedPath()[0].item);
      }

      _hasSelectedPerson(person) {
        return !!person;
      }
    }

    customElements.define(PeopleTemplate.is, PeopleTemplate);
  </script>

</dom-module>

You can also check it on plnkr.co where it's wrapped in a plain index file to see it actually running.

So, to summarize:

1. Any specific comments on my use of computed?
2. Any generic comments on this component?

Answer 1

Any specific comments on my use of computed?

I understand it's an annoying thing to have to repeat the dependencies for computed function but that's how Polymer can infer when that property has to be refreshed. If you had to use only the function name then it had to analyze your JavaScript code and it's an definitely a complex/slow task (it can't simply execute it to keep track of what you're reading because code may have side effects and/or multiple execution paths). The easiest workaround is to have you explicitly name the dependencies. Notably View.js does it for you.

Think positive: at least you do not have to add observers to all the dependencies and to manually invalidate the computed property...

Any generic comments on this component?

Few things you can improve. First of all a non primitive default value shouldn't be an immediate value but a function. That value instance is shared across all instances of your component. Imagine you have two instances of <people-template> and in one of them you push() a new item in people. That new item is added to all the default instances! You should simply rewrite is as:

people: {
    type: Array,
    value: function () {
        return [
            { id: 1, name: "John Doe" },
            { id: 2, name: "Mary Havenstein" },
            { id: 3, name: "Janie van Staffelen" }
        ];
    }
}

There are, of course, cases when this shared value behavior is what you need (for example to implement a custom iron-meta-like component).

---

You set a new value for the property using this.set(), it's perfectly allowed but in this simple case you may also just use:

this.selectedPerson = event.composedPath()[0].item;

Of course you may decide to mark selectedPerson as read-only, in this case keep this.set() in-place.

---

I find that getting the model using composedPath()[0] is little bit fragile. dom-repeat template lets you use an easier syntax (see also Data binding helper elements):

this.selectedPerson = event.model.item;

Note that item is the name of the item used in the dom-repeat template, in your case you specified another name with as="person" then you need to use it:

this.selectedPerson = event.model.person;

---

I find person != null much more clear than return !!person if person is not supposed to be a boolean value.

---

In general I'd suggest to use a prefix for your custom components and to drop -template. Reasoning: they're all templates then you repeat the obvious and using a prefix you highly reduce possible collisions with 3rd party components.

---

You have a computed property hasSelectedPerson you use to hide a paragraph. Few things might be optimized here:

• dom-if template does not need a boolean property, any falsy value will do the job then simply if="[[selectedPerson]]" is sufficient.
• dom-if to hide a single element without any complex logic (for example lists) is an overkill and a (small) performance hit. You can use the hidden attribute (eventually complimented by a specific *[hidden] CSS rule when there isn't browser support) to achieve the same result: <p hidden$="[[selectedPerson]]">...</p>.