Skip to content
This repository was archived by the owner on Dec 15, 2022. It is now read-only.

Conversation

@nathansobo
Copy link
Contributor

@nathansobo nathansobo commented Feb 23, 2017

Previously, Etch wrapped the virtual-dom library to perform DOM updates. This PR eliminates that dependency and handles DOM reconciliation directly in Etch itself, simplifying various aspects of the implementation in the process.

The reconciliation code in our new patch function is largely based on Snabbdom's implementation with some tweaks to support Etch's features.

Implementing our own reconciliation makes Etch simpler in multiple ways:

  • We now support components directly and straightforwardly rather than relying on virtual-dom's Widget system. If the "tag name" of a virtual node is a function, we perform a component update, which assumes the component follows Etch's uniform component interface and has an update method.
  • Refs are now supported directly, eliminating the need to maintain a global stack of refs objects.
  • Binding arbitrary events via addEventListener is also now supported via an events property pointing to a hash of (event name, listener) pairs, with an automatic this binding on the parent component.

I haven't formally benchmarked this change, but it should also be more performant. Snabbdom, the library on which this reconciliation solution is based, performs well in benchmarks. The big difference from the previous implementation is that patch operates on DOM nodes directly rather than producing a diff object to apply later. Early usage in Atom's text editor and find and replace implementation are showing solid performance.

Remaining tasks:

  • Support SVG elements and properties (thanks @as-cii)
  • Iron out bugs discovered using this in a real application
  • Resolve conflicts with master
  • Move source files to lib now that Babel is out of the picture and remove src from .npmignore
  • Update README to talk about events and ensure nothing else has changed

Nathan Sobo and others added 30 commits October 8, 2016 22:13
Implementation is cribbed from snabbdom, but pruned down a bit.
Still need to test all the cases.
In all these places, the node can be a text node, which isn't technically an element.
Support the same set of events that React supports.
Signed-off-by: Nathan Sobo <nathan@github.com>
Signed-off-by: Nathan Sobo <nathan@github.com>
Signed-off-by: Nathan Sobo <nathan@github.com>
@as-cii as-cii force-pushed the custom-virtual-dom branch from d8f2022 to 827f311 Compare February 23, 2017 08:41
src/patch.js Outdated
}
}

if (newProps && newProps.key) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm somewhat worried about the overhead of always copying the props object. I deliberately avoided doing it in other places such as populated the events sub-object. Thinking about our alternatives...

  • We could mutate the props object we are given. This is somewhat more dangerous than the events case because we're losing information at that point and if the props object gets reused somehow it could be trouble.
  • We could only clone the object and delete the key when passing props down to a component, then explicitly ignore the key in our normal DOM patching operations.
  • We could change the logic and just pass the key through to the component. It would be the component's responsibility to ignore it. This seems maybe the simplest and fastest to me but could require changes to existing application code using previous Etch versions.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants