Closed theengineear closed 5 years ago
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
@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!
Caught something while fiddling -- we should probably throw a warning if you declare an observer that points to a non-existent function
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.
Note: reflection should happen before observation.
Should we resolve functions at runtime? Then we can compute a class-level function which can be cached for each class?
@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.
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.
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 , 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.
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?
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.
@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?
@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.
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 👍
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.
@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.
Changes:
Non-changes:
*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
element-mixin
: Provides base functionality.properties-mixin
: Allows you to declare theproperties
block.property-effects-mixin
: Allows you to declare effects that should happen when a property changes.lit-html-mixin
: Chooseslit-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
withoutproperties-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:
type
is just used for serialization/deserialization)Importantly, the following order of operations for initialization is meant to match:
Then, for each subsequent property change:
Handling dependency graphs
Consider the following properties:
The graph looks like this:
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 ifa
changes, you need to then updateb
and then updatec
, in that order.Vernacular
The following terms are thrown around in the code:
setup
: This is construction-time work.initialization
: This happens during the firstconnectedCallback
.analysis
: This is property-centric and is a static analysis done as prerequisite for property initialization