Skip to content
This repository has been archived by the owner on Dec 31, 2020. It is now read-only.

@observer(['store props']) not available to react-dnd #77

Closed
jamiewinder opened this issue Jul 6, 2016 · 11 comments
Closed

@observer(['store props']) not available to react-dnd #77

jamiewinder opened this issue Jul 6, 2016 · 11 comments

Comments

@jamiewinder
Copy link
Member

I had posted this as a comment in #64 , but I think it may be an issue in its own right rather than a question of usage.

When using @observer([stores]) with react-dnd, the injected props aren't available to the react-dnd functions.

const dragSourceSpec = {
    beginDrag(props) {
        const appState = props.appState; // appState is undefined
        // ...
    }
};

const dragSourceCollect = (...);

@DragSource('item', dragSourceSpec, dragSourceCollect)
@observer(['appState'])class MyItem extends Component {
     // ...
}

Swapping the decorators around fixes this, but causes @observer to have no effect other than the props injection.

Let me know if any more details are needed.

@jtraub
Copy link
Contributor

jtraub commented Jul 6, 2016

@jamiewinder would you mind attaching a minimal example which reproduces the problem?

@jamiewinder
Copy link
Member Author

jamiewinder commented Jul 6, 2016

@jtraub - Here you are:

http://codepen.io/jamiewinder/pen/PzKPko?editors=1010

As you can see on the rendered component, the appState is reaching the component during rendering (as are the normal props). However, if you begin dragging the text, you'll get an alert ('appState is undefined, ticks is x') showing that the @observer-injected prop is unavailable, but normal ones are.

If you swap the decorators around on TestComponent and try again, you'll get the expected alert ('appState is 123, ticks is x') but because the @observer decorator isn't in the inner-most, it'll no longer be a reactive component and won't be updating in response to the ticks incrementing.

@jtraub
Copy link
Contributor

jtraub commented Jul 6, 2016

@jamiewinder http://jsbin.com/yayuzedusi/edit?js,output

Add another observer on top or use something else (react-tunnel for example) to pass appState to the component which is produced after DragSource application.

You can even pass stores manually

@jtraub
Copy link
Contributor

jtraub commented Jul 6, 2016

Note: I do not have react-dnd experience at all.

As far as I understood beginDrag will be called when dragging starts. The function will receive props which were passed to the component produced by let TestComponent = DragSource('test', dragSourceSpec, dragSourceCollect)(observer(['appState'])(TestComponent)) call.

This component receives only clock prop (see render method of TestApp). appState is injected into props of real TestComponent by observer(['appState']) function and it happens inside.

@jamiewinder
Copy link
Member Author

Thanks, I just stumbled on the same workaround! I think you're probably right in your guess at what the cause is, but I'm not sure what needs to be done about it. I go with the workaround for now.

@jtraub
Copy link
Contributor

jtraub commented Jul 6, 2016

It is expected HOC behavior and it is documented in README.md

observer will pick up the named stores from the context and make them available as props of the decorated component

In your case decorated component is class TestComponent ... observer provides appStore to this component. I believe @mweststrate can not do anything to change props of parent component: React simply won't allow it.

I would recommend you to install React Developer Tools and see an actual component hierarchy in your example to better understand how several decorators are applied to a component.

@jamiewinder
Copy link
Member Author

It might be expected, but applying the same decorator twice is a bit strange. It feels a bit like @observer is perhaps doing double duties - providing stores, and making the component reactive? The following may be a little more clear?

@provide(['appState'])
@dragSource
@observer class MyComponent {
}

That said, I'm not sure there are many other situations where you'd want to @provide something to a non-reactive component, so I can see why it was added to @observer. The requirement to do these two things separately may well be limited to usage with react-dnd...

What do you think @mweststrate ?

@jtraub
Copy link
Contributor

jtraub commented Jul 7, 2016

Technically it is quite easy to provide a separate decorator inject or provide.

Mobx-react already has such function which is used internally.

Given, that all you need to do is to slightly modify and export this function under an appropriate name, It would be great to have this feature.

@mweststrate
Copy link
Member

@jtraub yes I think you are right. I think we should export a separate inject or connect HoC, (and keep the current observer(["fluff"]) as convenience?). I'll try to update this coming weekend (unless anybody else beats me to it ;-)).

@mweststrate
Copy link
Member

See also: #70

@mweststrate
Copy link
Member

Just released 3.5.0, which separates inject from observer. That should provide the tools to fix the above issue. Should not be possible to solve this as follows:

@inject('appState')
@DragSource('item', dragSourceSpec, dragSourceCollect)
@observer
class // ... erc

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

No branches or pull requests

3 participants