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

React Komposer 2.x #123

Closed
arunoda opened this issue Sep 27, 2016 · 11 comments
Closed

React Komposer 2.x #123

arunoda opened this issue Sep 27, 2016 · 11 comments

Comments

@arunoda
Copy link
Owner

arunoda commented Sep 27, 2016

We've been pretty busy with some internal apps these days. Sorry for not updating.
When we are building it and looking at some of these issues, I think we need to re-think about the API.

Here's what we are looking to have in 2.x (Which will be out pretty soon).

import { makeComposer, merge } from 'react-komposer';
// Additional dataLoader generators are located in a different NPM package.
// These goes into the same repo. But multiple NPM modules. That's to have their own NPM modules for each of these.
import getPromiseLoader from 'react-komposer-promise';
import getRxLoader from 'react-komposer-rx';

// We need to this once in an app.
// With this, we could create our own base compose utility with our own options.
// Additionally, this'll allow us to use react-komposer under React Native since
// we don't have any loading and error components.
const compose = makeComposer({
  loading: () => (<div>Loading..</div>),
  error: (error) => alert(error.message),
  pure: true,
  propsToWatch: [], // Now dataLoader is re-run only when one of the props mentioned is changed. In this case, it only runs once.
  context: { // This is how we pass addition stuff to the dataLoader.
    reduxStore: {}
  }
});

// Now there's two callbacks onData and onError.
// That's because, we don't usually use onError.
// We may handle it via our own component rather using something global.
// We use the onError specially in prototyping the app.
const dataLoader = (props, onData, onError) => {
  // Fetch from the data.
  // We can access the context via this.
  //   For an example, we can get the reduxStore with `this.reduxStore`
  return aa.subscribe();
};

const now = Rx.Observable.interval(1000)
  .map(() => new Date().toString());

// Now composeAll is renamed into merge
const DataAwareComponent = merge(
  compose(dataLoader, { propsToWatch: ['appId'] }), // We could alter options like this.
  compose(getPromiseLoader(Promise.resolve(100), 'promiseData')),
  compose(getRxLoader(now, 'date')),
)(<Component/>);
@arunoda
Copy link
Owner Author

arunoda commented Sep 27, 2016

We should also look at documenting how to use recompose if needed.

@terry-fei
Copy link
Contributor

Expecting

@karldanninger
Copy link

Nice @arunoda! Looking forward to it

@clayne11
Copy link
Contributor

clayne11 commented Oct 4, 2016

I'm HIGHLY opposed to using this to access context. Why not pass it in as an argument?

For that matter, I think the args that are passed in should be an object rather than ordered parameters:

const dataLoader = ({props, onData, onError, context}) => {
  // Fetch from the data.
  return aa.subscribe();
};

This API is much cleaner and you can simply ignore the arguments that you don't use. With the example you give using arrow functions the this context wouldn't even be set properly. Using this adds quite a bit of surface area for bugs.

@arunoda
Copy link
Owner Author

arunoda commented Oct 5, 2016

@clayne11 yep. I agree and I found it confusing specially with the arrow functions.
So, now I am using it as you are describing.
It's now on v2 branch and we are using it on production.

@terry-fei
Copy link
Contributor

How soon will v2 merge to master?

@ElegantSudo
Copy link

Hm, I think the old pattern/usage was simpler.

@macrozone
Copy link

by the way, react-komposer share some functionality with https://github.com/acdlite/recompose (like composeAll / merge is mostly the same like recompose' compose). Did you think about using it under the hood?

@arunoda arunoda mentioned this issue Oct 21, 2016
@arunoda
Copy link
Owner Author

arunoda commented Nov 8, 2016

Edit: I just released it.

Guys, v2 is almost ready and I've added docs as well.

@arunoda
Copy link
Owner Author

arunoda commented Nov 8, 2016

@macrozone we only use a single feature of recompose which is merge. It's a few lines of code and I hope the API is bit different too.

We use react-komposer for data loading, but after that it's possible to use recompose or use recomposed UI components with react-komposer.

@arunoda
Copy link
Owner Author

arunoda commented Nov 8, 2016

Closing since I've released it. But feel free to open new issues for questions.

@arunoda arunoda closed this as completed Nov 8, 2016
Repository owner locked and limited conversation to collaborators Nov 8, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants