artsy/react-redux-controller

is it compatible with React 0.15?

Opened this issue · 11 comments

I have warnings about manually validating PropType:

Warning: You are manually calling a React.PropTypes validation function for the `routing` prop on `Controller`. This is deprecated and will not work in production with the next major version. You may be seeing this warning due to a third-party PropTypes library. See https://fb.me/react-warning-dont-call-proptypes for details.

@plandem - can you give a little more context on where this is happening?

@erikdstock probably it does not matter now, because:
a) we don't use ramda, so I had to fork and replace ramda with lodash
b) we needed a way to handle errors from generators and there is opened issue(#8) about this for ages, so I had to fix it.
c) after all these changes - I can't reproduce this warning anymore.

P.S.: if you want, you can check commits here:
https://github.com/plandem/react-redux-controller

acjay commented

We're using with React 0.15 at Artsy with no warnings. react-redux-controller doesn't call any PropTypes validations, it just shuttles them between objects. I'm guessing it's happening elsewhere. My guess was maybe react-redux, but they've got a similar issue where they say they're not at fault (reduxjs/react-redux#484). But clearly you've gone through all the code, so feel free to correct me if you see that call happening someplace.

Glad you're finding value with the project, and cool to see you've extended it.

@acjay I would like to use your npm package directly, but have no idea how to replace ramda with lodash without any headache. The only way - replace these 3 functions(map/flatten/pick) with own implementation :(

acjay commented

I'm all for Lodash, although I'm partial to the lodash-fp API flavor, since it preserves the immutability and data-last aspects of Ramda. (I don't really think we really put data-last to work in this project, but I was convinced of the merits by things such as the links under Introductions on the Ramda home page.)

  1. lodash/fp is based on lodash
  2. I already replaced ramda with lodash and as result only 3 functions from lodash are using: mapValues, flattenDeep, pick. Actually, I don't see any reason for lodash-fp here - result already immutable in case of these functions.

But why do you need data-last aspect?

acjay commented

Yeah, same project, just a different "flavor", interface, or however you want to call it :)

Yep, looking at how you did the the conversion to Lodash, I agree that you're effectively working immutably. But one thing I appreciate about the lodash/fp is that everything is immutable to begin with, whereas with the original Lodash, it varies across the API, so there's no need to reason about it at all. I've gotten bitten in the past with subtle mutations of references passed into a helper method.

As I said earlier, we're not currently leveraging data-last to any real advantage. So I agree that it's a toss-up here. It's just my sense that it's the better default stylistic choice, because it scales better to more interesting usages. If you haven't checked out the links Ramda references, you might find some of them compelling!

I can create a separate branch with this changes to help you :)

acjay commented

That sounds great, thanks!

I'm hoping to have some time to put some maintenance into this project soon :) It's minimal enough that it doesn't need that much love, but I'd still like to make a few tweaks.

will try to commit today

P.S.:
I really like your concept of controller/sync(for async) actions more than redux-saga or based on it dva.

before I used vanilla redux and was trying to find way to improve/decouple some part of application. Your solution is tiny yet powerful....with vanilla redux same time.

@acjay, I created pull request for branch with lodash, #13

p.s.: only one helper function disaggregateSuperSelector is not refactored - I never used it and there is no usage at library, so I omit it.