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

Attach all functions to an object in order to hold config options #2

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

freshtonic
Copy link

This is much cleaner that cracking open almost every function to pass
along the skipPropsWithNonDefaultValues option.

This is much cleaner that cracking open almost every function to pass
along the `skipPropsWithNonDefaultValues` option.
@rogeriochaves
Copy link
Owner

Hello, @freshtonic!

Thank you so much for contributing! And sorry for the long delay on this, I was very busy (what a poor excuse). I liked your idea!

But, about the code, I don't want to go in that direction, when the options grows, the conditionals inside the main file will grow as well (and this file is already too big, I might split it on a future refactor).

Also, I don't want to use classes, as this is being kinda of a functional programming exercise for me, and as you can see classes add a lot of boilerplate, the number of lines of code grew a lot. I want to use simple pure functions as much as I can.

So, the solution I achieved is a bit different, I commited it this other PR: #3

The interface also changed, so now instead of working with optionals parameters, we really compose functions to achieve this:

decompile(withoutDefaultProps(<Bar/>))

Or, if you are using a library like Ramda (which I love):

compose(decompile, withoutDefaultProps, withAnotherFutureOption)(<Bar />)

(considering that future options can be added by only changing input or output, of course)

What you think?

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.

2 participants