Skip to content

Conversation

taion
Copy link
Contributor

@taion taion commented Nov 5, 2015

No description provided.

@omnidan
Copy link
Contributor

omnidan commented Nov 5, 2015

@taion I'm a bit confused - why is this a better way to do it? I always thought it was just a preference thing

@taion
Copy link
Contributor Author

taion commented Nov 5, 2015

It's a little bit inelegant with react-mixin still because you have to manually bind the routerWillLeave hook. It's definitely up to user preference in the end, but on further thought and from talking to @ryanflorence, I think given a choice, I'd lean very slightly toward recommending using React.createClass over react-mixin to users who need advice on this in the first place.

I'm personally going to use react-mixin, but I think users checking these docs would be better served by dropping to React.createClass.

@omnidan
Copy link
Contributor

omnidan commented Nov 5, 2015

Ah, that totally makes sense then - it's also good to avoid using brand-new syntax (decorators are ES7) for general understanding. Thanks for clearing it up and the PR 😁

omnidan added a commit that referenced this pull request Nov 5, 2015
Prefer React.createClass over react-mixin
@omnidan omnidan merged commit 9ed71de into master Nov 5, 2015
@taion taion deleted the taion-patch-1 branch November 5, 2015 16:55
@taion
Copy link
Contributor Author

taion commented Nov 5, 2015

Yup - I think we still want to point users at react-mixin, just... with a bit less enthusiasm.

@omnidan
Copy link
Contributor

omnidan commented Nov 5, 2015

@taion yeah, maybe just note it: "Note: If you're using ES7 decorators, you can try out react-mixin" or something like that

@taion
Copy link
Contributor Author

taion commented Nov 5, 2015

Well, the react-mixin thing works regardless. If users are comfortable/familiar with it, they'll know what to do. If not, then they probably should use createClass.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants