atom / etch

Builds components using a simple and explicit API around virtual-dom
MIT License
555 stars 57 forks source link

How to move an Etch component's child? #69

Open illright opened 6 years ago

illright commented 6 years ago

Consider the following scenario:
I have a root component that holds some rows. I also have a button that should be a child of the latest row at all times.

On the root component creation it holds a single row which in turn holds the button, so the inital markup looks like this:

<Root>
  <Row ref="row0">
    <Button ref="btn" />
  </Row>
</Root>

Then at some point I append another Row inside Root and I want the markup to look like this:

<Root>
  <Row ref="row0"></Row>
  <Row ref="row1">
    <Button ref="btn" />  // this should point to the same DOM node as the previous Button
  </Row>
</Root>

Here's my attempt at doing this:

class Button {
  constructor() {
    etch.initialize(this);
  }

  render() {
    return (
      <button>Test</button>
    );
  }

  update() {
    return etch.update(this);
  }
}

class Row {
  constructor(props, children) {
    this.children = children;
    etch.initialize(this);
  }

  render() {
    return (
      <div>
        {this.children}
      </div>
    );
  }

  update() {
    return etch.update(this);
  }
}

export default class Root {
  constructor() {
    this.otherRows = [];
    this.rowCount = 1;
    etch.initialize(this);
  }

  render() {
    return (
      <div>
        <Row ref="row0">
          <Button ref="btn" />
        </Row>
        {this.otherRows}
      </div>
    );
  }

  addRow() {
    const lastRow = this.refs[`row${this.rowCount - 1}`];
    const button = lastRow.children.pop();
    const newRow = new Row({}, [button]);

    this.otherRows.push(newRow);
    this.update();
    this.rowCount++;
  }

  update() {
    return etch.update(this);
  }
}

And when I call the addRow method after Root's establishment, I get the following markup:

<div> // This is the Root
  <div>  // This is a Row
    <button />
  </div>
  <undefined>
    <button />
  </undefined>
</div>

And it seems like the two buttons that appear in the markup are different DOM nodes.
How do I achieve my goal?

BinaryMuse commented 6 years ago

Just at a glance, there are a couple small issues here.

First, the addRow function of Root is incorrect. You're instantiating a new Row instance, and adding it to the array, but that's not what <Row /> does when you write it (e.g. in your render function) — the JSX just describes the creation of a new Row. To fix this, you'd do something more like const newRow = <Row>{button}</Row>.

However, I still think that won't result in the output you're looking for. You're mutating the children prop for lastRow by calling lastRow.children.pop(); you should treat props (including children) as immutable, and instead modify state further up the tree to result in the proper JSX being returned.

Said another way: your components' render method should be a pure function of props and any component-local state. The article in the React docs called Thinking in React can help explain this if you're not used to working with this sort of abstraction. You may also be interested in some stuff I wrote for a question on Stack Overflow: https://stackoverflow.com/a/23594466/62082

illright commented 6 years ago

@BinaryMuse Thanks, I see what you mean. Will it be a performance hit to recreate the button (and bind the event handler) every time a new row is appended? And if yes, then how can I manage that?

Also, another thing that bothers me is the way of getting the actual Button component instance. It seems like the refs.btn can only be accessed from an individual Row and not from Root, so I need a way to access rows. The current approach I have (refs with indexes) seems very dirty and inefficient. What could you suggest here?

BinaryMuse commented 6 years ago

If you use keys when creating your rows in the array, then the button shouldn't be recreated — Etch should be smart enough to insert the new rows above it. As for the button, you really shouldn't need to access it; what are you trying to do that requires using the ref?

illright commented 6 years ago

@BinaryMuse can you please elaborate? The README has no examples of using keys (which are badly needed btw). I'd love to see a code sample.

And the thing is, the rows in my example do not contain just the button, they also have other children before the button (I omitted this fact to make the example clearer). And technically in my case the button is not even a button - it's a text field, I need a ref to access its content. So simply inserting a row above is not an option. Sorry if those facts should have been mentioned earlier, I just tried my best to abstract away the details :)