4
\$\begingroup\$

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?
\$\endgroup\$
3
\$\begingroup\$

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>.
\$\endgroup\$
2
  • \$\begingroup\$ Thanks for your insightful remarks. One question about the event.model.item remark: I actually implemented it like that at first (and variations of it) but could not get it to work at all. E.g. if you make only that change to my plnkr you can see that clicking items won't function anymore... \$\endgroup\$ – Jeroen Oct 24 '17 at 11:22
  • \$\begingroup\$ It's just because you named item person with as="person". Name of the property (item) reflects that name. Change the code to e.model.person (updated my answer as well) \$\endgroup\$ – Adriano Repetti Oct 24 '17 at 11:36

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.