Preact: dangerouslySetInnerHTML, children and setState quirks

[molily]

Hello,

first of all, thank you for this great project. After having built several React apps, we’ve decided to give Preact a try. We’ve built an app using preact-cli and almost everything worked flawlessly. I’m really excited about this project!

We ran into one strange issue that took me hours to track down. This is the smallest test case I was able to produce:

import { h, render, Component } from 'preact';
import PropTypes from 'prop-types';

class Container extends Component {

  constructor() {
    super();
    this.state = { state: 'initial' };
  }

  componentDidMount() {
    setTimeout(() => {
      this.setState({ state: 'updated' });
    }, 2000);
  }

  render() {
    const { children } = this.props;
    const { state } = this.state;
    return (
      <div>
        <div>Container state: {state}</div>
        {children}
      </div>
    );
  }

}

Container.propTypes = {
  children: PropTypes.element.isRequired
};

const App = () => (
  <Container>
    <div>App</div>
    <div>
      →
      <span dangerouslySetInnerHTML={{ __html: '<u>HTML</u>' }} />
      ←
    </div>
  </Container>
);

render(<App />, window.document.body);

Expected behavior: After the state change, the content set via dangerouslySetInnerHTML remains unchanged.

Actual behavior: After the state change, the content set via dangerouslySetInnerHTML is removed from the DOM completely.

I’m a newbie to the codebase so I can explain why this happens but I cannot come up with a fix.

Under normal circumstances, when the state of a component changes, the call stack is rerender → idiff → innerDiffNode (which removes the subtree added by dangerouslySetInnerHTML) → diffAttributes → setAccessor (which sets innerHTML again).

Under normal circumstances, setAccessor is called because this condition …

attrs[name]!==(name==='value' || name==='checked' ? dom[name] : old[name]))

yields true, in short

attrs[name]!==old[name]

yields true. The old value (an object { __html: '<u>HTML</u>' }) is a equal but not identical to the new value (also an object { __html: '<u>HTML</u>' }). This is because the object is created anew with each App#render call.

In contrast, in the test case above, App#render is not called again when Container re-renders, so it’s the same old attributes object. Therefore diffAttributes does not detect a difference so it does not call setAccessor to set innerHTML again.

Hopefully this analysis from an outsider helps. My guess would be that we need a separate check for name === 'dangerouslySetInnerHTML'. Another possible solution I can think of is to not remove the subtree in the first place, since no change happened.

For reference, here’s the same example based on Facebook’s React:
https://codepen.io/anon/pen/BdqVWQ?editors=0010

[developit]

Hi there - thanks for the awesome detailed bug report!

I actually think this might come down to the inequality here being reversed:
https://github.com/developit/preact/blob/master/src/vdom/diff.js#L143

[molily]

Here’s a simple workaround for those who encounter this bug too and need a quick fix. Make sure to not use dangerouslySetInnerHTML in the component that passes children (App in my example) to the component that changes its state (Container in my example). Put it in a child component instead, e.g.:

const InnerHTMLHelper = ({ tagName, html }) =>
  h(tagName, { dangerouslySetInnerHTML: { __html: html } });

const App = () => (
  <Container>
    <div>App</div>
    <div>
      →
      {/* <span dangerouslySetInnerHTML={{ __html: '<u>HTML</u>' }} /> */}
      <InnerHTMLHelper tagName='div' html='<u>HTML</u>' />
      ←
    </div>
  </Container>
);

[developit]

Ah - neat fix. I'll try to include that in the durable fix as a test too!

[rmales]

I think this may be the same (underlying) issue as in https://github.com/developit/preact/issues/578

[simonjoom]

I ve got same problem of you on click on button to pop something updating state in innerhtml children
I resolved this by create this hack:

can you try my fix to resolve it and report to us if it's working as well for you?

in preact.js
change:
innerDiffNode(out, vchildren, context, mountAll, hydrating || props.dangerouslySetInnerHTML != null);
to
innerDiffNode(out, vchildren, context, mountAll, hydrating, props.dangerouslySetInnerHTML != null);

create 6args in innerdiffnode:
change that:
function innerDiffNode(dom, vchildren, context, mountAll, isHydrating) {
to
function innerDiffNode(dom, vchildren, context, mountAll, isHydrating,InnerHTML) {

then change:
} else if (!child && min < childrenLen)
to
} else if (!InnerHTML&&!child && min < childrenLen)

test and answer me thanks

[marvinhagemeister]

@molily We just released the alpha for our next version of Preact which doesn't recreate innerHTML on each render anymore. It checks if the html string has changed before proceeding further, just like you suggested :+1:

[horprogs]

I had the same problem, using preact 10.0.0-beta.3. Had we already released fix?

[marvinhagemeister]

@horprogs Can you share a codesandbox or a repo where we can reproduce it?

[horprogs]

Hmm.. I removed yarn.lock and node_modules, reinstall, and now it works fine.
I'm sorry to trouble you and thank you for your job!