Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dangerouslySetInnerHTML doesn't update on SVG <g> #2863

Closed
AsaAyers opened this issue Jan 15, 2015 · 13 comments
Closed

dangerouslySetInnerHTML doesn't update on SVG <g> #2863

AsaAyers opened this issue Jan 15, 2015 · 13 comments
Labels

Comments

@AsaAyers
Copy link

I suspect this applies to all SVG elements, but I only know <g> for sure. Due to lack of an SVG <image> I'm needing to use dangerouslySetInnerHTML. I created a jsfiddle to demonstrate the issue. This will switch between two images of Tux each second. It will also dump innerHTML to the console after each render.

Every browser renders correctly initially, but only Chrome can update.

  • Chrome: Everything works as expected
  • Firefox: the <image> gets converted to an <img> and disappears. I assume because it's now an HTML <img> inside an SVG context.
  • Safari: innerHTML looks fine, but the image never changes.
  • IE 11: innerHTML looks fine, but the image never changes.
@balanceiskey
Copy link
Contributor

We can confirm the same for use elements inside of SVGs in Safari. Firefox seems to work okay though.

@balanceiskey
Copy link
Contributor

Worth noting, we're using dangerouslySetInnerHTML in this case to utilize the xlink:href attribute on SVGs

@AsaAyers
Copy link
Author

I have a mixin I'm using to change the key on the parent every time the props change to force it to fully re-render. I did modify this before posting because my first version involved keeping the key with this.setState(), but that fell apparent when I had a component that actually used state.

// https://github.com/facebook/react/issues/2863
//
// React operates by touching the DOM as little as possible. Whenever it can
// it'll keep existing DOM nodes and just modify attributes. The strategy when
// using `dangerouslySetInnerHTML` is to simply set `.innerHTML` on the DOM
// node.  When it comes to <g> (and probably other SVG tags) setting .innerHTML
// doesn't work. But the strategy it uses to create them initially is fine.
//
// On any React element you can pass a key as a unique identifier.  If the
// first render says `<g key={1} />` and the next render says `<g key={2} />`
// then react knows that these are backed by different objects and you aren't
// just changing properties. By changing the key every time the image changes
// it forces React to throw away the `<g>` DOM node and create a new one.
//
//  http://facebook.github.io/react/docs/multiple-components.html#child-reconciliation
var workaround2863 = {
    get2863key: function() {
        var nextProps = this.props;
        var nextState = this.state;
        var lastProps = this._lastProps2863;
        var lastState = this._lastState2863;

        if (!_.isEqual(lastProps, nextProps)
        || !_.isEqual(lastState, nextState)) {

            this._cached2863Key = Date.now();
        }

        this._lastProps2863 = nextProps;
        this._lastState2863 = nextState;
        return this._cached2863Key;
    },
    exampleRender: function() {
        var html = '....';
        return (
            <g
                key={this.get2863key()}
                dangerouslySetInnerHTML={{__html: html}}
            />
        );
    }
};

@AsaAyers
Copy link
Author

I'm pretty sure there's a much simpler workaround:

render: function() {
    var html = "...";
    return (
        <g key={html} dangerouslySetInnerHTML={{__html: html}} />
    );
}

@sophiebits sophiebits added the SVG label Feb 27, 2015
@joernroeder
Copy link

I'm currently trying to update rect elements inside a 'svg' tag and went into the same problem:
<rect transform="translate(' + this.props.x + ', 0)" /> works well in the following browsers and didn't update at all in (mobile) safari.

  • console (logs in render method)
  • chrome
  • firefox

@AsaAyers
Copy link
Author

AsaAyers commented Apr 6, 2015

Rect is a supported tag. Why are you using dangerouslySetInnerHTML?

@joernroeder
Copy link

@AsaAyers I'm using <clipPath> elements and attributes as well which aren't supported at the moment: #2030

@joernroeder
Copy link

Updating the key manually forced react to rerender the svg and fixed my problem in safari
<rect key={this.props.x} ... />

@AsaAyers
Copy link
Author

AsaAyers commented Apr 6, 2015

Interesting. I ran into that exact problem on Friday. It works as a Style though:

                <circle
                    style={{
                        // passing clipPath as a property didn't work, but
                        // using a style is fine.
                        clipPath: "url(#keep-left)"
                    }}

@joernroeder
Copy link

good workaround for the clip-path attribute! But I got stuck with the <clipPath> element as well (which isn't a supported svg element https://facebook.github.io/react/docs/tags-and-attributes.html#svg-elements) and found this post on stackoverflow where I came accross the dangerouslySetInnerHTML solution.

@Dangoo
Copy link

Dangoo commented May 22, 2015

For everybody who comes to this issue from now:
There's a quite sweet solution for a lot of SVG attributes - just use the CSS version.
List of available properties

Example:

// This doesn't work in Safari
<svg dangerouslySetInnerHTML={
    { __html: `<path d="${this._getPath(path1)} ${this._getPath(path2)}" fill-rule="evenodd"></path>`}
}></svg>

// This does
<svg>
    <path d={this._getPath(path1) + ' ' + this._getPath(path2)} style={fill-rule: evenodd;}></path>
</svg>

It even reads better ;)

@ArnaudRinquin
Copy link

My case is a bit different, I download SVG content (images) and inline them this way:

render() {
    const { content } = this.props
    if (!content) return null

    return <g dangerouslySetInnerHTML={{ __html: content }}/>
  }

Dead simple. It works on Chrome and most recent browsers but it doesn't in IE10 nor IE9. I tried the key hack (both using content as key or @AsaAyers's trick) without success.

@ArnaudRinquin
Copy link

Good news everyone! This issue has been fixed few days ago

@gaearon gaearon closed this as completed Oct 27, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants