Problems using renderSubtreeIntoContainer

[slorber]

I don't know if this is the correct way to integrate this new "hidden feature" but I used.

var renderSubtreeIntoContainer = require("react-dom").unstable_renderSubtreeIntoContainer;

This is used on an existing app migrating from 0.13 to 0.14. I have tooltips that are portals (using Hubspot/DropJS) in which I render React components. The rendered content make use of React context. As context this has changed I can't use owner content anymore and must use renderSubtreeIntoContaine to make the context available. (see #4081)

At first I got a stacktrace from ReactBrowserEventEmitter.ensureScrollValueMonitoring because ReactBrowserEventEmitter.ReactEventListener was undefined. I don't know why I don't get it anymore, and my tooltips are efficiently displayed (at first I was requiring "react/lib/renderSubtreeIntoContainer" but don't think this is a reason right?)

But then my tooltips are displayed and works fine.
These tooltips can be hovered and clicked to trigger actions. This also works fine.

But there's a strange thing now: I've found a tooltip that, when hovered, produce a lot of errors. The tooltip that produces these errors is the on the first item of a list (for example, imagine a todo list, and the first todo have an attribute with a tooltip on hover -> only this tooltip causes the errors).

The stacktrace is:

Uncaught Error: Invariant Violation: 
ReactMount: Two valid but unequal nodes with the same `data-reactid`: .0.0 appLibs.js:59549
invariant appLibs.js:59549
getID appLibs.js:52190
handleTopLevelWithPath appLibs.js:51122
handleTopLevelImpl appLibs.js:51073
Mixin.perform appLibs.js:57834
ReactDefaultBatchingStrategy.batchedUpdates appLibs.js:49769
batchedUpdates appLibs.js:56046
ReactEventListener.dispatchEvent

Any idea?

[slorber]

see also #4081

My full integration:

'use strict';

var React = require("react");
var _ = require("lodash");

var $ = require("jquery");

var TetherTooltip = require("tether-tooltip");

var WithLongHoverBehavior = require("common/withLongHoverBehavior");

// See https://github.com/facebook/react/issues/4081
// See https://github.com/facebook/react/pull/4184
// See https://github.com/facebook/react/issues/4301
var renderSubtreeIntoContainer = require("react-dom").unstable_renderSubtreeIntoContainer;


var ValidTooltipPositions = [
    'top left',
    'left top',
    'left middle',
    'left bottom',
    'bottom left',
    'bottom center',
    'bottom right',
    'right bottom',
    'right middle',
    'right top',
    'top right',
    'top center'
];

var TooltipConstraints = [
    {
        to: 'window',
        attachment: 'together',

        // Can be important because tether can switch from top to bottom, or left to right,
        // but it does not handle correctly bottom-left to bottom-right for exemple
        // Using pin will at least make the tooltip stay on the screen without overflow
        // (but there's a CSS bug that makes the tooltip arrow hidden by the content I think)
        pin: true
    }
];

/**
 * A wrapper to set around components that must have a tooltip
 * The tooltip knows how to reposition itself according to constraints on scroll/resize...
 * See http://github.hubspot.com/tooltip/
 */
var WithTooltip = React.createClass({
    propTypes: {
        // The children on which the tooltip must be displayed on hover
        children: React.PropTypes.node.isRequired,
        // The prefered position (by default it will try to constrain the tooltip into window boundaries
        position: React.PropTypes.oneOf(ValidTooltipPositions),

        // The tooltip content (can be an inlined HTML string or simple text)
        // If not defined, the tooltip will be disabled
        content: React.PropTypes.node,

        // Permits to disable the tooltip
        disabled: React.PropTypes.bool,

        // Wether this tooltip can be hovered or not (useful if the tooltip contains buttons)
        hoverable: React.PropTypes.bool
    },


    isDisabled: function() {
        if ( this.props.disabled ) {
            return true;
        }
        else if ( !this.props.content ) {
            return true;
        }
        else {
            return false;
        }
    },


    // TODO can probably be optimized?
    resetTooltipForCurrentProps: function() {
        // The timeout is required because otherwise TetherTooltip messes up with animations entering (ReactCSSTransitionGroup)
        // TODO find why! is there a better solution?
        setTimeout(function() {
            if (this.isMounted()) {
                this.destroyTooltip();
                if ( !this.isDisabled() ) {
                    this.tetherTooltip = new TetherTooltip({
                        target: React.findDOMNode(this),
                        position: this.props.position || 'bottom left',
                        content: " ", // Disable as we manage the content ourselves
                        // See https://github.com/HubSpot/tooltip/issues/5#issuecomment-33735589
                        tetherOptions: {
                            constraints: TooltipConstraints
                        }
                    });
                    if ( this.props.hoverable ) {
                        $(this.getTetherTooltipNode()).addClass("tooltip-hoverable");
                    }

                    // We mount the tooltip content ourselves because we want to be able to mount React content as tooltip
                    var tooltipContentNode = $(this.getTetherTooltipNode()).find(".tooltip-content")[0];
                    if ( React.isValidElement(this.props.content) ) {
                        renderSubtreeIntoContainer(this, this.props.content, tooltipContentNode);
                    }
                    else {
                        tooltipContentNode.innerHTML = this.props.content;
                    }
                }
            }
        }.bind(this),0);
    },

    componentDidMount: function() {
        this.resetTooltipForCurrentProps();
    },
    componentDidUpdate: function(previousProps) {
        var positionHasChanged = (this.props.position !== previousProps.position);
        var contentHasChanged = (this.props.content !== previousProps.content);
        var disabledHasChanged = (this.props.disabled !== previousProps.disabled);
        var childrenHasChanged = (this.props.children !== previousProps.children);
        var hasChanged = positionHasChanged || disabledHasChanged || contentHasChanged || childrenHasChanged;
        if ( hasChanged ) {
            this.resetTooltipForCurrentProps();
        }
    },
    componentWillUnmount: function() {
        this.destroyTooltip();
    },

    destroyTooltip: function() {
        if ( this.tetherTooltip ) {
            this.tetherTooltip.destroy();
            delete this.tetherTooltip;
        }
    },

    getTooltipTarget: function() {
        if (typeof this.props.children === 'string') {
            return <span>{this.props.children}</span>;
        } else {
            return React.Children.only(this.props.children);
        }
    },

    // It may return nothing if the tooltip is already removed from DOM
    getTetherTooltipNode: function() {
        return this.tetherTooltip && this.tetherTooltip.drop && this.tetherTooltip.drop.drop;
    },

    onLongHover: function() {
        $(this.getTetherTooltipNode()).addClass("long-hover");
    },
    onHoverEnd: function() {
        $(this.getTetherTooltipNode()).removeClass("long-hover");
    },

    render: function() {
        return (
            <WithLongHoverBehavior longHoverDelay={5000} onLongHover={this.onLongHover} onHoverEnd={this.onHoverEnd}>
                {this.getTooltipTarget()}
            </WithLongHoverBehavior>
        );
    }

});

module.exports = WithTooltip;

[jimfb]

@slorber Nothing jumps to mind. Your code appears to be pretty complex. If you believe it's an issue with React, it would be helpful if you can provide a minimal jsfiddle that demonstrates the issue. This example imports a bunch of libraries, and has a bunch of things going on in there.

Also, if you'er debugging, it may be worth looking at what nodes are at id '.0.0'; the value of those nodes may provide you with some context/info that makes it more obvious what's going on there.

[slorber]

@jimfb will have to take a closer look but actually I noticed this error with other cases too, not involving the hovering of the tooltip or renderSubtreeIntoContainer.

For now we won't use the beta1 as it has a more serious issue (#4288)

[jimfb]

@slorber Yeah, a simple jsfiddle would still be helpful so we can fix this issue :P.

[slorber]

Unfortunatly I'm not even sure it's related to my tooltip code so don't really know how to reproduce that in a sandbox.

All I can say is that after migrating, we have these console errors (but it does not seem to impact the UI as far as i've seen). The tooltip code has not changed (except usage of renderSubtreeIntoContainer instead of React.render because we need to forward parent props).
Reverting to 0.13.3 resolved this problem.

Would this help if you get access to my migrated app with NODE_ENV != production and source maps? I can host it somewhere

[dcousens]

@slorber this still relevant? Close?

[slorber]

@dcousens I think this is still relevant in React 0.14 but did not try again as the beta1 has bugs so I'll give more insights on this bug when beta2 or RC is released

[jimfb]

Realistically, this bug isn't too actionable in it's current state. A minimal jsfiddle would help us identify the issue.

Might be related to #3790, since that's the only place I'm aware of that results in this error being incorrectly thrown, but it's just hard to tell from this.

[slorber]

@jimfb I'll try to provide more infos when I can

Actually I'm not even sure it has something to do with renderSubtreeIntoContainer

What I can tell is that I did not have this error in 0.13.3 and it's only in the 0.14.0-beta1

[jimfb]

Yeah, it's hard for me to imagine how renderSubtreeIntoContainer could be the cause of the issue, since (in theory) the only thing it does differently from React.render is to propagate context. My guess is that it is either user error (ie. one of your libraries is manually modifying dom nodes) or is related to a race condition in React ala #3790.

[zpao]

FWIW, I shipped beta2 last night so you might be able to get a repro case going.

[sophiebits]

Closing unless we hear more.

[slorber]

Yes I'll come back to you if I still have the problem but currently I don't have time to try agaiin the 0.14 migration