babel-plugin-transform-incremental-dom icon indicating copy to clipboard operation
babel-plugin-transform-incremental-dom copied to clipboard

Capital DOM nodes are Components

Open jridgewell opened this issue 8 years ago • 17 comments

Currently, we're doing the following:

// Correct
<div id="test" />
elementVoid("div", null, ["id", "test"]);

// Incorrect
<Component id="test" />
elementVoid("Component", null, ["id", "test"]);

Notice, we've turned a Component into the string literal "Component". In proper JSX, capital DOM nodes are kept as Identifiers:

// Correct
<Component id="test" />
elementVoid(Component, null, ["id", "test"]);

jridgewell avatar Jul 28 '15 22:07 jridgewell

How do we want to handle this? Because React has a different code path for these types of components which will instantiate a class.

jamiebuilds avatar Jul 28 '15 22:07 jamiebuilds

I'm thinking this is tied to https://github.com/google/incremental-dom/pull/66#issuecomment-121853982. Given a component, we need to ensure that the a placeholder exists for it. Then, invoke the component.

<Component key={key} id="test" param={param} />

// - - -
var _tmp = elementPlaceholder("div", key, ["key", key]); // Notice no "id" attribute.
Component(_tmp, { id: 'test', param: param });

There are a few liberties here. First, I disagree strongly with attributes being set on the placeholder element. I think React has nailed the parameter passing syntax to components, it's the only thing that makes sense in a template based view language like JSX. Personally, I think it's fine if we diverge from iDOM's first-class API for setting attributes in favor of JSX's parameter passing.

Second, we invoke the the Component instead of newing it. We could new, but that's a throw-away object every time we render. I think the smarter option is for Instances to be setup by the user directly, passing in some function (or #mount/#render methods for complicated Components) that does the job of replacing _tmp/mutating/patching it as a container.

I need to look more into how React handles child Components, though.

jridgewell avatar Jul 28 '15 22:07 jridgewell

Second, we invoke the the Component instead of newing it. We could new, but that's a throw-away object every time we render.

I thought React worked by holding onto instances of Component in between cycles, and only shutting them down if they were removed. Is that not a possibility here?

jamiebuilds avatar Jul 28 '15 22:07 jamiebuilds

I thought React worked by holding onto instances of Component in between cycles

That would be the ideal goal, but we don't have the information available to do it currently. A component helper function like the kind @sparhami mentioned would get us most of the way (management of Component instance), but there's not currently a way to tell if the a Component needs to be created (first render), is created (second render), or is being removed.

jridgewell avatar Jul 29 '15 00:07 jridgewell

@jridgewell @thejameskyle I managed to create a React-esque Component that can determine whether it's the initial render or subsequent render by tapping into incremental-dom's alignWithDOM (internal)...

Here's how I'm using it... https://github.com/peterwmwong/incremental-dom-experiments/blob/1aaac51681429ae5b514a3e6d7a4ed0ab6208d2f/src/helpers/StatefulComponent.js#L76

I have a mostly working (very WIP) TodoMVC app using this... https://github.com/peterwmwong/incremental-dom-experiments/blob/master/src/components/App.jsx

It would be nice if a public API of similar nature were exposed by incremental-dom.

peterwmwong avatar Jul 29 '15 04:07 peterwmwong

I don't have any solution knowing when the Component is removed... I would need hook in incremental-dom's traverse exitNode()

peterwmwong avatar Jul 29 '15 04:07 peterwmwong

tapping into incremental-dom's alignWithDOM (internal)

Yah, we could definitely do this with overridden elementOpen (store last created node), elementPlaceholder (grab the parents private __incrementalDOMData and detect if component is present in some map) and elementClose (traverse children and drop any missing components defined in the private DOMData), but it'd all be a hack.

A public API is the only good solution here. Maybe a callback argument to patch, which gets called with create, update, delete events and the respective node?

jridgewell avatar Jul 29 '15 05:07 jridgewell

I thought about adding hook for node creation / removal to the library, but decided not to for now as it is also possible with just standard JavaScript. For example:

var createElement = Document.prototype.createElement;
var removeChild = Node.prototype.removeChild;

Document.prototype.createElement = function() {
    var node = createElement.apply(this, arguments);
    // do something with node
    return node;
};

Node.prototype.removeChild = function() {
    var node = removeChild.apply(this, arguments);
    // do something with node
    return node;
};

One limitation is that all calls to createElement and removeChild, even those not initiated by Incremental DOM would be captured and you would need to filter the ones that you are interested in.

sparhami avatar Jul 29 '15 05:07 sparhami

but decided not to for now as it is also possible with just standard JavaScript

Monkey patching a native class just feels so icky. :frowning:

Would you be open to PR adding lifecycle hooks?

jridgewell avatar Jul 29 '15 05:07 jridgewell

Yeah, I would sooner monkey patch iDOM than native objects.

jamiebuilds avatar Jul 29 '15 05:07 jamiebuilds

I'd be open to allowing the createElement function from nodes to be monkey patched. A removeChild function could also be added to nodes.

sparhami avatar Jul 29 '15 06:07 sparhami

This is really a must have fix

asafyish avatar Oct 01 '15 18:10 asafyish

See #17. The beginning work is there, but we still need some translateComponentToTagName in iDOM to fully support this.

jridgewell avatar Oct 01 '15 18:10 jridgewell

Is there a relevant pull request in incremental dom ?

asafyish avatar Nov 03 '15 06:11 asafyish

since #43 is merged does this now work?

apaleslimghost avatar Jan 26 '18 12:01 apaleslimghost

You need to build wrappers around iDOM functions to support it, since it's not natively done yet.

jridgewell avatar Jan 27 '18 00:01 jridgewell

Hello :)! I don't really understander how to use or defind component in JSX. Even thought I set "components": true in .babelrc. My code is: render () { const MyComponent = () => <div>Hello World</div>;return <MyComponent</MyComponent>} It doesn't work. Then I try: const MyComponent = <div>Hello World</div> and: const MyComponent = () => {return <div>Hello World</div>} All are not work :( I want to know how to wirte a component?

waif1989 avatar Nov 15 '18 15:11 waif1989