Skip to content

Conversation

andreypopp
Copy link
Owner

No description provided.

@andreypopp andreypopp requested a review from TrySound December 20, 2017 11:04
"react": "^15.2.1",
"react-derivable": "../../",
"react-dom": "^15.2.1"
"react-derivable": "link:../.."
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, is it realtime update? Nice!

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not quite as src/ should be built into lib/.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still better then reinstalling

Value: {counter.get()}
<div>Value: {r(counter)}</div>
<div>
Is Even: {renderMap(counter, counter => (counter % 2 === 0 ? 'Yes' : 'No'))}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought you don't like map. Maybe renderDerive?:)

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't create a derivation so I thought it's better not to call it *Derive.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it

*/

let state = atom({
let state = Derivable.atom({
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found using der prefix quite nice and small.

der.atom
der.derive

Component = decorateWith(Component, makeReactiveComponent);
return decorateWith(Component, makePureComponent);
export function render<V: React.Node>(d: Derivable.Derivable<V>): React.Node {
return <Reactive d={d} f={renderValue} />;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Somebody would like to use this component directly :)

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know those guys from new context rfc

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you elaborate?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some guys from context rfc are jealously prefer jsx over function calls and hocs.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it matter? I think we'd like to settle on what React Context API officially settles.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This style seems to me less noisy than using JSX with function-as-children.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't force. Just a note. I'm not a fan of pure jsx too.

@andreypopp
Copy link
Owner Author

This doesn't yet work on server as reactors should not be initialized there.

@andreypopp
Copy link
Owner Author

Also not sure if we need to add some kind of pure API (a-la PureComponent)... Right now it seems not required as it's easy just to do:

return renderMap(d, v => <SomePureComponent={v} />)

@TrySound
Copy link
Collaborator

Yep, PureComponent won't give wins here.

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