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

Explain unit testing decorated components. #607

Merged
merged 4 commits into from
Aug 26, 2015

Conversation

ghengeveld
Copy link
Contributor

Also improved test setup a bit using a babelhook file rather than a command line option, for more flexibility.

Using the babelhook file is more flexible because it allows configuration such as setting the experimental stage.
@gaearon
Copy link
Contributor

gaearon commented Aug 23, 2015

Wouldn't babel/register use your .babelrc? I think it would, and IMO you should keep all your options in .babelrc anyway so they're centralized.

@eugene1g
Copy link

Exporting a default and a named module is a great approach, however there is a caveat that it only works with no changes if the consuming side loads the Redux component via ES6 import and not via CommonJS require.

Babel handles the interchangeable use of ES6 import and CJS require through its interop capability to run two module formats side-by-side, but the behaviour is slightly different. Essentially, if all you have is export default connect(...) then babel overrides module.exports for you so that CJS var App = require('./App.js') gets you the default decorated component. As soon as you add a named export, the require fails and you need to change the consuming CJS side to be var app = require("./app.js").default. See a sample example In the upcoming Babel V6, handling of the default export might change so that CJS code will always have to use require("./App.js").default so that the behaviour is normalised, but for now it remains a gotcha.

The reason why it's relevant is that people might be using ES6 modules + Babel to build their front-end code, but still use CommonJS format in testing or server-side code/rendering. So the same component might be consumed in several places through different module systems. Right now in their CJS side var App = require("./App.js") works fine, but if they add a named export inside App.js then consumers will break and will need to be updated to var App = require("./App.js").default

ES6 is the clear winner long term and the suggested testing technique is very valuable, however there should be a note warning people that if they use CommonJS to require ES6 Redux smart components, then they should explicitely access it via require('SmartApp.js').default even if all they have is a default module. This is not idiomatic CommonJS, but it should continue working in the future even if Babel changes its behaviour or the module starts exporting other named piece of code.

@ghengeveld
Copy link
Contributor Author

@gaearon Fair point. I've reverted this change.
@eugene1g Thanks for your feedback. I have no experience with mixing module systems, so I'm happy you provided such clear explanation. I'll include a note on this in the docs.

gaearon added a commit that referenced this pull request Aug 26, 2015
Explain unit testing decorated components.
@gaearon gaearon merged commit e5b2a5b into reduxjs:master Aug 26, 2015
@gaearon
Copy link
Contributor

gaearon commented Aug 26, 2015

This is in, thanks

@ghengeveld
Copy link
Contributor Author

Yesterday I had a chat with someone suggesting to simply unwrap the decorated component instead. He suggested using something like this to recursively unwrap it and use that in unit tests:

function unwrap(component) {
  return component.wrapped ? unwrap(component.wrapped) : component;
}

I haven't tried this approach myself yet, but it seems a bit cleaner than having multiple exports, especially since you don't have to change your application sources to support your unit tests. How do you guys feel about this?

@gaearon
Copy link
Contributor

gaearon commented Sep 13, 2015

That would need every HOC to converge on the same property name. In reality I think it works worse than just exporting vanilla stuff.

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

Successfully merging this pull request may close these issues.

None yet

3 participants