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

container props and default done #229

Merged
merged 2 commits into from
Mar 27, 2015
Merged

Conversation

quazzie
Copy link
Contributor

@quazzie quazzie commented Mar 26, 2015

Did not listen to propchanges (componentWillReceiveProps).
default for done was using an fat-arrow function. Accessing this in fat-arrow will bind it to the initial component.
The call later to done with .call(config, ..) did nothing because it was already bound.

Did not listen to propchanges (componentWillReceiveProps).
default for done was using an fat-arrow function. Accessing this in fat-arrow will bind it to the initial component.
The call later to done with .call(config, ..) did nothing because it was already bound.
getInitialState() {
config = _.defaults(config, {
// Have a default implementation of done so it can be
// called from other handlers
done: (results) => {
done: function (results) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we revert this back to the ES6 function declaration please

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we expect this to be the config object

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But it isn't. With ES6 fat arrow when you call done.call(config, results) it does not use the config as this because when getInitialState is called the function is bound to the component.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, i'm an idiot. you are right. sorry!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its the initial component even. So if you have unmounted it and then mounted it THIS inside default done is the first now unmounted component. With the old props and such.

@chrisbarmonde
Copy link

Ah yeah, I think this just got me. Had a module with module.exports = container(React.createClass({... but all the components I created ended up with the same props.

@quazzie
Copy link
Contributor Author

quazzie commented Mar 27, 2015

@chrisbarmonde Yes =) but i didn't create multiple instances, just one that got mounted and unmounted a couple of times. I always got the same props and i passed in a callback when the callback got called it was on an old unmounted parent.

But i've been wondering, if i export a container and use it multiple times on one page would it work ? Wouldn't all the components share props and such because all share same config ?

@quazzie quazzie mentioned this pull request Mar 27, 2015
jhollingworth added a commit that referenced this pull request Mar 27, 2015
container props and default done
@jhollingworth jhollingworth merged commit b88977a into martyjs:master Mar 27, 2015
@quazzie quazzie deleted the patch-1 branch March 27, 2015 08:16
@jhollingworth
Copy link
Contributor

This has been release in v0.9.2. Thanks for your help!

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.

3 participants