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

connect not running on on exported component class #163

Closed
rockingskier opened this issue Oct 19, 2015 · 12 comments
Closed

connect not running on on exported component class #163

rockingskier opened this issue Oct 19, 2015 · 12 comments

Comments

@rockingskier
Copy link

To be honest I'm not sure if this is the right repo but I'm at an impasse and this is the most obvious place for now.

I have a simple test repo. It is heavily based off the redux-router basic example but simplified for this example.

Among other webpack-esque things the following react/redux libs are installed:

  "dependencies": {
    "history": "^1.12.5",
    "react": "^0.14.0",
    "react-dom": "^0.14.0",
    "react-redux": "^4.0.0",
    "react-router": "^1.0.0-rc3",
    "redux": "^3.0.2",
    "redux-router": "^1.0.0-beta3"
  },

Fairly standard stuff.

There are two files worth looking at:

index.jsx - Basically a whole application
App.jsx - The main component exported to a separate file.

When run in one file, index.jx the application work as expected. The component receives props from the @connect-ed store, all good and happy. Routes run, props are passed, everyone gets what they want.

In the name of splitting things out for a real project I started by moving the main App component to a separate file. When I do this however the @connect-ing doesn't appear to work.
This is first made clear by the Props warning that appears.

Warning: Failed propType: Required prop `routerState` was not specified in `App`. Check the render method of `RoutingContext`.

This points to redux-router however after adding some logging I can see that the @connect mapStateToProps function is not being called when exporting the component.

This all seems very odd to me but tbh it could point to any number of places. The eco system is moving very quickly and its hard to keep up.

Is there an issue with react-redux, @connect, webpack, babel, redux-router or any of the (incredible) hot reloading features?

@gaearon
Copy link
Contributor

gaearon commented Oct 20, 2015

Thanks for reporting. Seems like the same issue as gaearon/babel-plugin-react-transform#37 so I'll close it. You can work around it by renaming the import until it's fixed. I'll come back to this issue after fixing the underlying one to confirm that it worked.

@gaearon gaearon closed this as completed Oct 20, 2015
@rockingskier
Copy link
Author

Glad to see someone has caught the issue and its not just me going mad.

MY aim is to not actually have the inline App Component so what exactly am I renaming as I cant see what would be conflicting.

@gaearon
Copy link
Contributor

gaearon commented Oct 20, 2015

You want to have two components named "App", one imported from App and another declared in index? The Babel plugin has a bug where it is confused by named import with the same name as something declared in the file. If you remove App from index it will work fine. But of course you can also disable Babel plugin until it is fixed. :-)

@rockingskier
Copy link
Author

Just the one App.

Defining the class inline with @connect works fine.

Moving the class to a separate file, still with @conenct, does not run the mapStateToProps.
Importing the so-called @conenct-ed App and then passing it through connect(fn)(App) again works. Obviously now what I want to achieve.

But yes, removing "plugins": ["react-transform"], from .babelrc works fine so I'll do that.

Thanks! Great work as always!

@adamscybot
Copy link

I have this issue but I dont have react-transform. My components are in different files -- es6 classes with export default. These are imported in various places. Also noticed that @connect doesnt seem to get executed in that neither the state or actions are available.

@gaearon
Copy link
Contributor

gaearon commented Nov 4, 2015

@adamscybot

Language features don't just "stop working" out of the blue. :-)

It's not something we can or should fix in the library. Most likely you're using decorator incorrectly but it's hard to say without code. Please create a StackOverflow question with code example and we will be happy to help. And try not using decorator syntax, instead following examples in the docs.

It is unrelated to this issue.

@adamscybot
Copy link

@gaearon

Indeed, the problem was actually because (after moving to babel 6) I was loading babel-plugin-syntax-decorators but not babel-plugin-transform-decorators. This setup doesn't throw any errors/warnings when it comes across a decorator.

For other reasons, it looks like decorators are broken anyway: babel/babel#2702

But as you mentioned this is unrelated. Cheers!

@gaearon
Copy link
Contributor

gaearon commented Nov 4, 2015

Got it, thanks for the explanation!

@gaearon
Copy link
Contributor

gaearon commented Mar 5, 2016

OK, so this is actually much more fun than I thought it was.

@rockingskier Not sure whether you’re still curious but here’s what happened.

  1. You were using both react-hot-loader and babel-plugin-react-transform which is something I missed initially. I wouldn’t say it’s unsupported straight away but this definitely isn’t something we test for.
  2. Because of this, both the component wrapped by connect() and the component returned by connect() were wrapped with react-proxy.
  3. Proxy tries to be smart to prevent double-wrapping. It sets a hidden field on the class to look up its “current” proxy.
  4. Now comes the fun part. connect() tries to be smart and hoists all static fields to the wrapping component including the unfortunate proxy field. So now proxies will think that connect()ed component has already been wrapped in a proxy, but will point to the inner component’s proxy. 😭
  5. Nothing works.

I’ll look if I can fix it on react-proxy side. In the retrospect hoisting all statics including non-enumerable ones was a silly decision, so we need to reconsider this in connect().

The quick fix is to avoid using react-hot-loader and react-transform-hmr in a single project 😄 .

gaearon added a commit to gaearon/react-proxy that referenced this issue Mar 5, 2016
gaearon added a commit to gaearon/react-proxy that referenced this issue Mar 5, 2016
gaearon added a commit to gaearon/react-proxy that referenced this issue Mar 5, 2016
gaearon added a commit to gaearon/react-proxy that referenced this issue Mar 5, 2016
gaearon added a commit to gaearon/react-proxy that referenced this issue Mar 5, 2016
@gaearon
Copy link
Contributor

gaearon commented Mar 5, 2016

I confirmed https://github.com/rockingskier/redux-problem as fixed by updating to react-proxy@1.1.4.

@gaearon
Copy link
Contributor

gaearon commented Mar 5, 2016

@thejameskyle would so love this.

@jamiebuilds
Copy link

tries not to gloat

...

IN YO FACE ABRAMOV!!!

(jk I luv u buddy)

gregberge pushed a commit to gaearon/react-hot-loader that referenced this issue Oct 29, 2017
manishsharma004 pushed a commit to manishsharma004/react-hot-loader that referenced this issue Jun 2, 2019
webgurucan added a commit to webgurucan/react-hot-loader that referenced this issue Mar 26, 2022
rubydev1211 added a commit to rubydev1211/react-proxy that referenced this issue May 19, 2022
dluoulb pushed a commit to dluoulb/hs7-react-hot-loader that referenced this issue Jan 31, 2024
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