-
Notifications
You must be signed in to change notification settings - Fork 180
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Initial API for updating attributes and properties. #332
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really dislike this "buffer", "queue", and "apply" naming. Even from the perspective of working on iDOM, it's super complicated (what is the difference between "buffer" and "queue" and how do I remember that without re-reading the damn code every time?).
/** | ||
* @type {function(!Element, string, *)} | ||
*/ | ||
const queueAttribute = queueChange.bind(null, applyAttr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Function#bind
is considerably slower than using a closure.
bufferAttribute, | ||
bufferProperty, | ||
queueAttribute, | ||
queueProperty, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we exposing the queues here?
* Immediately applies any diffs for the current element and buffered | ||
* attributes. | ||
*/ | ||
const applyUpdates = function() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: rename it to applyBuffer
if we're calling it bufferAttribute
and bufferProperty
?
How would static attributes/properties work under this model? Are we removing them entirely? |
Closing in favor of #408, might revisit splitting attributes and properties in the future. |
This flow is faster than
elementOpenStart()
+attr()
+elementOpenEnd()
while allowing consumers of the library to be explicit rather than needing to specify a hook. This also exposes the ability to simply queue changes and flush them later, which can be beneficial in high mutation cases.