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

Use parent-context if owner-context is undefined ? #3392

Closed
slorber opened this issue Mar 12, 2015 · 5 comments
Closed

Use parent-context if owner-context is undefined ? #3392

slorber opened this issue Mar 12, 2015 · 5 comments

Comments

@slorber
Copy link
Contributor

slorber commented Mar 12, 2015

Hi,

I'm a library author (https://github.com/stample/atom-react) trying to migrate to 0.13.

I call React.withContext outside of React around a top-level layout component, so it seems I'm in the special case of parent != owner as there is (I guess), no parent for my top-level component.

As React.withContext is deprecated, I'm trying to replace it with a top-level wrapper component.

var ChildContextProvider = React.createFactory(React.createClass({
    displayName: "ChildContextProvider",
    childContextTypes: {

        locales: React.PropTypes.any.isRequired,
        messages: React.PropTypes.any.isRequired,
        appContext: React.PropTypes.any.isRequired,

        atom: React.PropTypes.any.isRequired,
        publishEvent: React.PropTypes.any.isRequired,
        publishEvents: React.PropTypes.any.isRequired,
        publishCommand: React.PropTypes.any.isRequired,
        addEventListener: React.PropTypes.any.isRequired,
        removeEventListener: React.PropTypes.any.isRequired
    },
    getChildContext: function() {
        return this.props.context;
    },
    render: function() {
        return React.DOM.div({children: this.props.children});
    }
}));


function renderWithContext(component,domNode,context) {
    var componentWithContext = ChildContextProvider({children: component, context: context});
    React.render(componentWithContext,domNode);
}

renderWithContext(component,node,reactContext);

This does not work:

Warning: Failed Context Types: Required context `atom` was not specified in `LayoutSection`. warning.js:48
Warning: Failed Context Types: Required context `publishCommand` was not specified in `LayoutSection`. warning.js:48
Warning: Failed Context Types: Required context `publishEvents` was not specified in `LayoutSection`. warning.js:48
Warning: Failed Context Types: Required context `addEventListener` was not specified in `LayoutSection`. warning.js:48
Warning: Failed Context Types: Required context `removeEventListener` was not specified in `LayoutSection`. warning.js:48
Warning: owner-based and parent-based contexts differ (values: `undefined` vs `function () { [native code] }`) for key (addEventListener) while mounting LayoutSection (see: http://fb.me/react-context-by-parent) warning.js:48
Warning: owner-based and parent-based contexts differ (values: `undefined` vs `[object Object]`) for key (atom) while mounting LayoutSection (see: http://fb.me/react-context-by-parent) warning.js:48
Warning: owner-based and parent-based contexts differ (values: `undefined` vs `function () { [native code] }`) for key (publishCommand) while mounting LayoutSection (see: http://fb.me/react-context-by-parent) warning.js:48
Warning: owner-based and parent-based contexts differ (values: `undefined` vs `function () { [native code] }`) for key (publishEvents) while mounting LayoutSection (see: http://fb.me/react-context-by-parent) warning.js:48
Warning: owner-based and parent-based contexts differ (values: `undefined` vs `function () { [native code] }`) for key (removeEventListener) while mounting LayoutSection (see: http://fb.me/react-context-by-parent) 

To make it work, I have to use:

function renderWithContext(component,domNode,context) {
    var componentWithContext = ChildContextProvider({children: component, context: context});
    React.render(componentWithContext,domNode);
}

React.withContext(reactContext,function() {
    renderWithContext(component,node,reactContext);
}.bind(this));

A weird this is that the following, which is barely the same to me (providing an "owner"?), does not work:

function renderWithContext(component,domNode,context) {
    React.withContext(context,function() {
        var componentWithContext = ChildContextProvider({children: component, context: context});
        React.render(componentWithContext, domNode);
    });
}

renderWithContext(component,node,reactContext);

Summary

It seems to me there's a little problem when parent != owner.

  • Parent provided, owner not provided: context is not accessible as it still uses the owner. App complains a lot (warning owner = undefined VS parent = defined).
  • Parent not provided, owner provided: context is accessible and app works fine. App complains a lot (warning owner = defined VS parent = undefined).
  • Both provided: app works, there's only a deprecation warning for withContext

So in any case, I can't get rid of all warnings and I'm forced to keep using a deprecated feature.

And the best case (providing both owner context = parent context) leads to keeping some code like this:

function renderWithContext(component,domNode,context) {
    var componentWithContext = ChildContextProvider({children: component, context: context});
    React.render(componentWithContext,domNode);
}

React.withContext(reactContext,function() {
    renderWithContext(component,node,reactContext);
}.bind(this));

I think maybe if owner context is never used in the whole app, maybe React could use parent context instead of owner context

@slorber slorber changed the title Use parent-context if owner-content is undefined ? Use parent-context if owner-context is undefined ? Mar 12, 2015
slorber added a commit to stample/atom-react that referenced this issue Mar 12, 2015
@zpao
Copy link
Member

zpao commented Mar 12, 2015

Unfortunately you're right that there simply is no way to actually use the parent context yet. You're probably best off using the deprecated features until then. Context is a trickier migration because we want it to be this.context and that's taken until we cut over. The warnings we have now are mostly unactionable.

I really don't think we should mix the two and conditionally use parent or owner. It's fraught with peril, any dependency change could break everything.

#2112 is the issue we're using to track the switch over.

@JSFB @sebmarkbage - anything to add?

@sebmarkbage
Copy link
Collaborator

There are two tricks you can use in your ChildContextProvider.

The simplest one is to just accept a function instead of passing the child in directly:

    render: function() {
        return this.props.createChild();
    }

So instead of:

renderWithContext(<MyComponent />, ...);

You would do:

renderWithContext(() => <MyComponent />, ...);

OR

You can use the legacy cloneWithProps method since that tool gives it a new owner and context. If you have nested elements, you would have to loop over them using ReactChildren, recursively.

    render: function() {
        return React.addons.cloneWithProps(this.props.child);
    }

This will make the context provider the owner of those children. That way it will work with both parent/owner based context.

The complexity comes from the fact that you're passing them in from outside the root. Usually they're created in the root itself. Again, this complexity is why we're changing things.

@slorber
Copy link
Contributor Author

slorber commented Mar 13, 2015

Thanks it works fine by passing a function to create the child, and I could remove the withContext call

@slorber slorber closed this as completed Mar 13, 2015
zordius pushed a commit to zordius/fluxex that referenced this issue Mar 14, 2015
natew added a commit to reapp/reapp-routes that referenced this issue Mar 16, 2015
@natew
Copy link

natew commented Mar 18, 2015

I'm getting mis-reads (The ReactMount TypeError: Cannot read property 'firstChild' of undefined error) when using this method alongside react-router on route transitions.

Because my factory component is at the top level, whenever I transition a route it is incrementing the root react-dataid. So for example it will start as .1 and then go to .2. But on certain route's I get that error as it increments.

Going to explore a little into why but wanted to note here first.

@natew
Copy link

natew commented Mar 18, 2015

Ok, I'm avoiding the top level factory pattern for now which works fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants