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

Add a compose export #3

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

RichardForrester
Copy link
Contributor

I've noticed that implementations of compose vary greatly and especially the libraries that support placeholders seem to fail on Builders.

I thought it would be better if njsx supplied it's own compose function so I added.

Let me know if you can try it out and if you have any issue with it.

@RichardForrester
Copy link
Contributor Author

RichardForrester commented Jul 21, 2019

Don't pull yet. I've noticed some artifacts from this compose function. I have a different one that works better with Builders and has better typescript support. I will commit as soon as I check everything out.

What do you think about the API?

It's not a true compose as in Haskell which only takes two arguments. It's really a pipeLeft which I abbreviate as pL in my projects. But for njsx, I was thinking composeBuilders, but that's too long.

@RichardForrester
Copy link
Contributor Author

RichardForrester commented Jul 21, 2019

Hey, I'm going to get back to this later tonight and add support for more arguments, I think 12 should be enough and change the implementation so that it does not need to be called, explained below.

I realized that you don't really want to supply the final argument to the compose function as you would normally. Basically want to list your nested Builders and just return a Builder. It can be done with this commit by calling the composition with (null):

import { composeBuilders } from 'njsx'

const Root = ({ store }) =>
  composeBuilders(
    Provider({ store }),
    PersistGate({ loading: null, persistor }),
    Router({ history }),
    Route({ path: '/', component: App })
  )(null)()

However, I think it would be better to not even have to call it with null. I've got to run and do family stuff now, but if you get a chance let me know what you think and I'll finish it up tonight.

I'm thinking nest would be a good name. So the final API might be something like:

import { nest } from 'njsx'

const Root = ({ store }) =>
  nest(
    Provider({ store }),
    PersistGate({ loading: null, persistor }),
    Router({ history }),
    Route({ path: '/', component: App })
  )()

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.

1 participant