Refactor x-element mixins.

[theengineear]

Changes:

1. Postpone more work until initial connect (closes XElement should not render until connected. #14 & closes No way to listen to errors dispatched during initialization. #22).
2. Upgrade all properties at initialization time (closes Upgrade all properties, not just attributes #15).
3. Initialize properties before initial render (closes Initialize properties before initial render is called #17).
4. Improve handling of property effects (closes Split apart property analysis from property initialization. #19).
5. Robustify test suite.

Non-changes:

1. This should remain a drop-in replacement for the old*†

*functionally this is the case, I've uncovered some previously-silenced errors though.
†note that reflection happens before observation as a result of this change.

Notes:

Mixin hierarchy

1. element-mixin: Provides base functionality.
2. properties-mixin: Allows you to declare the properties block.
3. property-effects-mixin: Allows you to declare effects that should happen when a property changes.
4. lit-html-mixin: Chooses lit-html as our templating engine.

It's currently the case that you can omit mixins from the tail, but things will break down if you choose to omit something from the middle/head. I.e., you cannot have property-effects-mixin without properties-mixin, but the other way around is OK.

Custom element conformance

Whatwg has a nice list of things which a custom element should conform to.

Polymer 3.0 behavior

We loosely conform to the behavior of Polymer since there is considerable road time on that interface. In general, we aim to provide some minimum subset of the behaviors found in that project. Here are a couple places where we still diverge:

1. Order of arguments in observer callback (this will be fixed later as it would case backwards incompatibility)
2. Computed properties are called when initial dependencies are undefined (this will be fixed later as it would case backwards incompatibility)
3. Type coercion happens on property set (this doesn't happen in Polymer where type is just used for serialization/deserialization)

Importantly, the following order of operations for initialization is meant to match:

1. constructor is called
2. minimal setup (just shadow root init)
3. initial connectedCallback
4. analyze properties
5. initialize properties (reconcile initial values and resolve computed properties)
6. initial render
7. initial reflection & observation

Then, for each subsequent property change:

1. reflect to attribute, if reflected
2. call observer, if it exists
3. compute dependent properties, if they exist (this can trigger further observations/reflections)

Handling dependency graphs

Consider the following properties:

{
  a: { type: Boolean },
  b: { type: Boolean, computed: 'computeB(a)' },
  c: { type: Boolean, computed: 'computeC(a, b)' }
}

The graph looks like this:

      a
   ↙     ↘
b     →    c

Previously, we were recursively walking the dependencies until some terminal condition was met. This change solves the graph up front so that future property-set operations are more deterministic.

Because our dependencies are really a directed, acyclic graph (DAG), the topological sorting algorithm will provide us with one (of many!) possible orders in which to traverse the edges of the graph. This allows us to move the burden of computed properties from set-time to setup-time.

For completeness, the solution to this graph is [a, b, c]. That means that if a changes, you need to then update b and then update c, in that order.

Vernacular

The following terms are thrown around in the code:

setup: This is construction-time work.
initialization: This happens during the first connectedCallback.
analysis: This is property-centric and is a static analysis done as prerequisite for property initialization

[klebba]

Great summary; might be nice to actually roll this into an artifact in the repository like SPEC.md or whatnot -- would have the added benefit of letting us discuss its contents in comments

[theengineear]

@klebba @charricknflx @codeStryke , I would love to get some feedback from you guys when you have a chance. This is a pretty big change to how x-element works--for the better, I hope!

[klebba]

Caught something while fiddling -- we should probably throw a warning if you declare an observer that points to a non-existent function

[theengineear]

Caught something while fiddling -- we should probably throw a warning if you declare an observer that points to a non-existent function

I believe we do this... no? It should dispatch an error event. I was just following that pattern we've had which is throw an event so that integrators can choose to do something with it or not. Otherwise, we set ourselves up to just be console spammers.

[theengineear]

Note: reflection should happen before observation.

[theengineear]

Should we resolve functions at runtime? Then we can compute a class-level function which can be cached for each class?

[klebba]

How about the case where:

const el = document.createElement('x-foo');
el.propA = 'value';
console.log(el.propB);

Where propB is computed with propA as input

[klebba]

Nit: can we organize this file into a collection of static methods on a class and then export that class

[theengineear]

sure, i'm indifferent, but I'll update this file according to the comments here.

[theengineear]

@klebba , I don't think we'll easily be able to support this use-case and support the guidelines provided to us by WHATWG. It clearly states that you are not allowed to inspect attributes or children during construction. That means that we cannot fully initialize our properties since they may be coming from attributes. And, if we cannot initialize our properties, we cannot perform computations. Now, the following will just work:

const el = document.createElement('x-foo');
el.propA = 'value';
document.body.appendChild(el);
console.log(el.propB);

I.e., you can do initialization of properties this way, but you cannot find computed results yet.

[codeStryke]

I did some corner case testing of the topological sort implementation and found that

topologicalSort({
    vertices: ['a', 'b', 'c', 'd', 'e', 'f', 'g'],
    edges: [['e', 'f'], ['e', 'h'], ['f', 'g'],['a', 'c'], ['b', 'c'], ['b', 'd'], ['c', 'e'], ['d', 'f']],
  })

will return

["b", "d", "a", "c", "e", "h", "f", "g"]

The vertex h doesn't exist in the vertices array in the input but does exist as an edge in the edges array.

Just putting it here for information, it may not be a real world problem.

[codeStryke]

Also an idea for the topological sort, can we mention/describe the algorithm we are using, for example https://en.wikipedia.org/wiki/Topological_sorting#Algorithms

[klebba]

@klebba , I don't think we'll easily be able to support this use-case and support the guidelines provided to us by WHATWG. It clearly states that you are not allowed to inspect attributes or children during construction. That means that we cannot fully initialize our properties since they may be coming from attributes.

Good point. This may convolute the design, but I do think we can compute properties in the constructor as long as we later apply the attribute values on attachment.

[klebba]

I found an issue (I think) where a computed property is called several more times than I would expect. The offending calls all have the same inputs; any idea what would cause this?

[theengineear]

I did some corner case testing of the topological sort implementation and found that...

Ah, thanks @codeStryke! I hadn't thought of that case. I think that won't happen, but we may as well guard against it 👍.

EDIT: on second thought, this shouldn't happen, so instead of checking for a condition that should never occur, I'm just going to leave a comment.

[theengineear]

@klebba , I don't think we'll easily be able to support this use-case and support the guidelines provided to us by WHATWG. It clearly states that you are not allowed to inspect attributes or children during construction. That means that we cannot fully initialize our properties since they may be coming from attributes.

Good point. This may convolute the design, but I do think we can compute properties in the constructor as long as we later apply the attribute values on attachment.

How would you handle computing something if you cannot initialize the value though? I.e., if we should not introspect attributes, we cannot be sure that our values have been initialized and our computations are correct. Right?

[theengineear]

@codeStryke , I based my algorithm off of an implementation in another language. I think I'll re-write with language closer to Kahn's algorithm to improve grok-ability for reviewers. Thanks for the nudge! I was implementing DFS, but I think implementing Kahn's algorithm might end up being easier to follow now that I'm revisiting.

[theengineear]

I found an issue (I think) where a computed property is called several more times than I would expect. The offending calls all have the same inputs; any idea what would cause this?

@klebba I'll look into this next time I have a moment to spend on this PR 👍

[charricknflx]

Clarify which properties do / don't work with each other?

can i have a "computed" property with a "value" property?

[theengineear]

I found an issue (I think) where a computed property is called several more times than I would expect. The offending calls all have the same inputs; any idea what would cause this?

@klebba I added a very specific test for this and am not seeing this behavior, let's sync up when I get this next revision through and see if we can get a reproducible test case.

[theengineear]

@klebba, I split #26, #27, #28, #29, #30, and #31 into separate tickets. I'd like to just minimize scope creep here so that we can get something merged in.

[theengineear]

This might warrant additional comment. In particular, we use this because we synchronously call render in the initialization phase and want to prevent unnecessary, duplicate rendering on initialization.