Change findTemplateNode to work on array of indices #4853
Conversation
| @@ -154,8 +150,7 @@ | |||
| * templateInfo: {object}, // nested template metadata | |||
| * // Metadata to allow efficient retrieval of instanced node | |||
| * // corresponding to this metadata | |||
| * parentInfo: {number}, // reference to parent nodeInfo> | |||
| * parentIndex: {number}, // index in parent's `childNodes` collection | |||
| * parentInfo: {Array}, // indices to traverse to this node from the template root | |||
There was a problem hiding this comment.
Unfortunately, this is a breaking change. I don't think we can accept this until Polymer 3.0.
There was a problem hiding this comment.
It may be possible to factor findTemplateNode such that it is still recursive, but able to operate either on a parentInfo tree (for backward compatibility) or a parentIndex array (for efficient serialization). We can consider this if we move forward with #4782 (or a non-breaking refactoring of it). In that way, the runtime code would still generate backward-compatible parentInfo trees (i.e. https://github.com/Polymer/polymer/pull/4853/files#diff-84d426316fb9e8502af8f5eed8675b40L286 would be reverted), and the build-time tool would transform that into an array of indices.
There was a problem hiding this comment.
It could certainly work by being able to parse both recursive notation as well as the array notation, but I don't think it would be good to land that before Polymer 3. Since the build-time tool is likely not ready before 3 is ready, landing it right now increases the Polymer code size and adds more conditions.
|
This is a good change, but it is (minorly) breaking. We'll leave it open for consideration for Polymer 3.x. |
| @@ -154,8 +150,7 @@ | |||
| * templateInfo: {object}, // nested template metadata | |||
| * // Metadata to allow efficient retrieval of instanced node | |||
| * // corresponding to this metadata | |||
| * parentInfo: {number}, // reference to parent nodeInfo> | |||
| * parentIndex: {number}, // index in parent's `childNodes` collection | |||
| * parentInfo: {Array}, // indices to traverse to this node from the template root | |||
There was a problem hiding this comment.
It may be possible to factor findTemplateNode such that it is still recursive, but able to operate either on a parentInfo tree (for backward compatibility) or a parentIndex array (for efficient serialization). We can consider this if we move forward with #4782 (or a non-breaking refactoring of it). In that way, the runtime code would still generate backward-compatible parentInfo trees (i.e. https://github.com/Polymer/polymer/pull/4853/files#diff-84d426316fb9e8502af8f5eed8675b40L286 would be reverted), and the build-time tool would transform that into an array of indices.
|
We have not incorporated this in 3.x and I think we have no plans of pursueing this any further. Therefore I am closing this PR. |
Originally part of the pre-building of effects, but that PR got too big to review. As such, I broke out this particular change which is completely stand-alone.
Previously, nested objects were used to represent how to retrieve a node in a template. It used a recursive pattern to find a parent. This solution only stores 1 integer per node and is iterative. This is especially a benefit when serializing this format, as it takes only 1 character to represent a node lookup instead of >20.