JSX inside IncrementalDOM.patch(el, callback) is removed

[jayphelps]

IncrementalDOM.patch(document.body, function () {
  <h1>suchwow</h1>
});

Works in v1.0.3

_incrementalDom2['default'].patch(document.body, function () {
  elementOpen('h1');
    text('suchwow');
  elementClose('h1');
});

but in v2.0.0 the JSX is simply removed, for noop

_incrementalDom2['default'].patch(document.body, function () {});

A quick glance makes me guess it was the aptly named: https://github.com/babel-plugins/babel-plugin-incremental-dom/commit/9b5faf28ade0bf22570f6cd969d1b44fcad7ad97 "Preemptively remove useless JSX"

[jayphelps]

Cc/ @jridgewell

[jayphelps]

Here's a demo repo: https://github.com/jayphelps/incremental-dom-experiments using v1.0.3 working. npm install babel-plugin-incremental-dom@2.0.0 and npm run start to demo it broken.

[jridgewell]

No need to CC, I'm watching the repo. :wink:

The same as in normal JSX, you'll need to return the JSX element for anything to happen.

IncrementalDOM.patch(document.body, function () {
  return <h1>suchwow</h1>;
});

[jridgewell]

And for context, we made this decision in #8. In normal JSX, element declarations (<div />) are non-destructive and essentially just primitive values. But with iDOM, declarations mutate the containing Element, so we wrap the "side" elements (things that are assigned to a variable, or occur outside the main JSX root element) in a wrapping function.

From that I identified that if we create a wrapper that is never called, there's no point in keeping the wrapper around. Hence, you're missing JSX elements.

We should probably update the README with a better #render example... Want to work on a PR?

[jayphelps]

@jridgewell Could have sworn I tried that too, but I'll give it another try cause clearly I didn't. I don't think transforming it to a noop is a great developer experience. If the code is noop, there's likely a bug and not having the translated code makes it appear like a bug with the plugin, rather than dev error. Just my 2 cents.

[jridgewell]

I don't think transforming it to a noop is a great developer experience. If the code is noop, there's likely a bug and not having the translated code makes it appear like a bug with the plugin, rather than dev error

Have a suggestion on how we can make this better? I really want this library to be easy to use, and my decisions aren't always the best since I'm so familiar with the code.

[jayphelps]

@jridgewell I can confirm that making it a return statement works.

To me, there are three alternative options:

• Emit iDOM as-is, with wrapper, let the dev figure out why it doesn't work
• Emit iDOM as-is, with wrapper, but emit a compile-time warning
• Emit a compile-time error, refuse to compile.

I'm not sure I have enough context to know which is ideal, but if there is truly no way for a person to use the emitted code, I would lean towards compile time error. That said, you can certainly do a similar thing with React-JSX; create virtual DOM that doesn't end up doing anything, and there's no warnings about that.

[jridgewell]

Ping @thejameskyle: I'm leaning towards compile time error. Thoughts?

[jayphelps]

@jridgewell Soooo now I can reproduce why it wasn't working even when I did return. In my app, I have another patch call inside the file, but it's inside a function that does not immediately invoke. To reproduce, I changed the demo, adding an example that induces this change: https://github.com/jayphelps/incremental-dom-experiments/blob/master/src/main.js#L11-L15

The compiled output is different as well.

Without second patch inside closure:

_incrementalDom2['default'].patch(document.body, function () {
  elementOpen('h1');
  text('suchwow');
  return elementClose('h1');
});

With second patch inside closure:

function _jsxWrapper(func) {
  func.__jsxDOMWrapper = true;
  return func;
}
_incrementalDom2['default'].patch(document.body, function () {
  return _jsxWrapper(function () {
    elementOpen('h1');
    text('suchwow');
    return elementClose('h1');
  });
});

window.onclick = function () {
  _incrementalDom2['default'].patch(document.body, function () {
    elementOpen('h1');
    text('pink power ranger');
    return elementClose('h1');
  });
};

I'm gonna step through the code and see if I can figure out why when they are wrapped, it doesn't work.

[jayphelps]

Seems to be simply because it returns yet another function, which iDOM doesn't do anything with.

exports.patch = function (node, fn, data) {
  enterContext(node);

  firstChild();
  // This is the call to the first callback, which returns the original function, uncalled.
  fn(data);
  parentNode();
  clearUnvisitedDOM(node);

  if (process.env.NODE_ENV !== 'production') {
    assertNoUnclosedTags(node);
  }

  getContext().notifyChanges();
  restoreContext();
};

[jridgewell]

Hmm, that's an unrelated issue. I'll open up another one to track that, leaving this one for what to do with an unused element.

[jamiebuilds]

@jridgewell I like the compile time error as well

[jayphelps]

Thanks fellas!

[jridgewell]

Thanks for the bug reports! Both issues have been fixed in v2.0.1.