jridgewell / babel-plugin-transform-incremental-dom

Turn JSX into IncrementalDOM
MIT License
144 stars 12 forks source link

Capital DOM nodes are Components #9

Open jridgewell opened 9 years ago

jridgewell commented 9 years ago

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"]);
jamiebuilds commented 9 years ago

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

jridgewell commented 9 years ago

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.

jamiebuilds commented 9 years ago

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?

jridgewell commented 9 years ago

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.

peterwmwong commented 9 years ago

@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 commented 9 years ago

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

jridgewell commented 9 years ago

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?

sparhami commented 9 years ago

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.

jridgewell commented 9 years ago

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?

jamiebuilds commented 9 years ago

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

sparhami commented 9 years ago

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

asafyish commented 9 years ago

This is really a must have fix

jridgewell commented 9 years ago

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

asafyish commented 9 years ago

Is there a relevant pull request in incremental dom ?

apaleslimghost commented 6 years ago

since #43 is merged does this now work?

jridgewell commented 6 years ago

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

waif1989 commented 6 years ago

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?