Skip to content
This repository has been archived by the owner on Apr 29, 2021. It is now read-only.

Capital DOM nodes are Components #9

Open
jridgewell opened this issue Jul 28, 2015 · 17 comments
Open

Capital DOM nodes are Components #9

jridgewell opened this issue Jul 28, 2015 · 17 comments

Comments

@jridgewell
Copy link
Owner

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
Copy link
Contributor

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
Copy link
Owner Author

I'm thinking this is tied to google/incremental-dom#66 (comment). 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
Copy link
Contributor

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
Copy link
Owner Author

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
Copy link

@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
Copy link

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

@jridgewell
Copy link
Owner Author

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
Copy link

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
Copy link
Owner Author

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

Monkey patching a native class just feels so icky. 😦

Would you be open to PR adding lifecycle hooks?

@jamiebuilds
Copy link
Contributor

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

@sparhami
Copy link

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
Copy link

asafyish commented Oct 1, 2015

This is really a must have fix

@jridgewell
Copy link
Owner Author

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

@asafyish
Copy link

asafyish commented Nov 3, 2015

Is there a relevant pull request in incremental dom ?

@apaleslimghost
Copy link

since #43 is merged does this now work?

@jridgewell
Copy link
Owner Author

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

@waif1989
Copy link

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?

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

No branches or pull requests

7 participants