Descriptor warning because willTransitionFrom receives descriptor, not instance

[sophiebits]

The transitions example currently gives this console warning when switching away from the dashboard page:

Invalid access to component property "refs" on Form at the top level. See http://fb.me/react-warning-descriptors . Use a static method instead:

.type.refs(...)

because of the line

if (component.refs.userInput.getDOMNode().value !== '') {

in transitions/app.js, but the router passes the component descriptor (basically just type and props) to the willTransitionFrom function.

In React master, I don't believe the example will work at all because descriptors and components have been more properly separated. If the intention is for willTransitionFrom to receive the actual component instance, your best approach is probably to use cloneWithProps in conjunction with new-style refs (facebook/react#1554) after that lands.

[mjackson]

Absolutely. I realize we're doing this wrong, and welcome any insight you can provide on how to do this correctly. The difference between component instances and descriptors has been one of the most difficult things to work with while building this library. We even have a few warnings in our tests because we're using descriptors instead of instances.

Looking over the cloneWithProps API, it doesn't look like it's intended to be used with descriptors. How should we get a handle on the real components?

[ryanflorence]

We could move willTransitionFrom to the actual component maybe?

Sent from my iPhone

On Jun 24, 2014, at 9:28 PM, Michael Jackson notifications@github.com wrote:

Absolutely. I realize we're doing this wrong, and welcome any insight you can provide on how to do this correctly. The difference between component instances and descriptors has been one of the most difficult things to work with while building this library. We even have a few warnings in our tests because we're using descriptors instead of instances.

Looking over the cloneWithProps API, it doesn't look like it's intended to be used with descriptors. How should we get a handle on the real components?

—
Reply to this email directly or view it on GitHub.

[ryanflorence]

had a conversation with @sebmarkbage, the only way to get a handle on the actual instance is the return value of React.renderComponent which we can't use, or refs.

I'm thinking we can maybe add a ref here, which can simply be the name of the route:

https://github.com/rpflorence/react-nested-router/blob/master/modules/components/Route.js#L442

And then instead of storing it on match, look it up on this.refs[routeName] when we pass the component in here:

https://github.com/rpflorence/react-nested-router/blob/master/modules/components/Route.js#L387

[sophiebits]

Right – with the new refs (not landed yet), you will be able to do something like:

var ref = React.createRef();
props.ref = ref;
match.handlerInstance = route.props.handler(props);

and then use ref.then(...) to resolve it into an actual component instance. (With the current refs, the component instance is attached as a ref to the component "owner", which is basically the deepest render call on the stack at the time of the descriptor's creation. We're switching the API because of situations like this where it's not obvious who the owner is.)

[mjackson]

@rpflorence Where would we get this.refs from? The way the code works currently, all we ever have is descriptors. I'm pretty sure if we say route.refs that we're not any better off than we are now.

Maybe we need a RouteHandler mixin.. but that requires you to say mixin: [Router.Mixin] on every single component you want to use in your routes, which is not ideal.

[ryanflorence]

We can bank on the root route I think, can't we?

On Wed, Jun 25, 2014 at 2:15 PM, Michael Jackson notifications@github.com
wrote:

@rpflorence https://github.com/rpflorence Where would we get this.refs
from? The way the code works currently, all we ever have is descriptors.
I'm pretty sure if we say route.refs that we're not any better off than
we are now.

Maybe we need a RouteHandler mixin.. but that requires you to say mixin:
[Router.Mixin] on every single component you want to use in your routes,
which is not ideal.

—
Reply to this email directly or view it on GitHub
#47 (comment)
.

[mjackson]

Not really. The current API delegates to users to call React.renderComponent, so all we ever see is descriptors.

If we went one level higher we could get a handle on the components themselves, or we could wait until the new refs @spicyj mentioned land in React proper.

[sophiebits]

all we ever see is descriptors

Not quite true – in the componentWillMount and componentDidMount hooks for the top-level Route, this refers to the Route component instance. If the Route's render method is the lowest on the stack when the descriptor that has a ref is created, the ref will be attached to that Route instance.

[mjackson]

@spicyj Ah, good to know. We may be able to work with that.

[WickyNilliams]

I'm getting this warning when checking state in my component's willTransitionFrom method. It looks like this fix hasn't landed in a tagged version yet. Are there plans to release any time soon, or is there going to be a cumulative update?

[ryanflorence]

thanks for the reminder to release something :)

I just published v0.2.1

On Mon, Jul 14, 2014 at 10:21 AM, Nick Williams notifications@github.com
wrote:

I'm getting this warning when checking state in my component's
willTransitionFrom method. It looks like this fix hasn't landed in a
tagged version yet. Are there plans to release any time soon, or is there
going to be a cumulative update?

—
Reply to this email directly or view it on GitHub
#47 (comment)
.

[WickyNilliams]

I was hoping it'd be a welcomed reminder :-)

Thanks, I'll test tomorrow.

[WickyNilliams]

Worked a treat, thanks