stampit-org / react-stamp

Composables for React.
370 stars 10 forks source link

Upcoming changes to the standard enable fluent syntax #30

Closed koresar closed 8 years ago

koresar commented 8 years ago

With the new composables specification the following:

const container = ({
  React,
  Button,
  Text,
}, ...behaviors) => (
  reactStamp(React).compose({
    state: {}
    render(){}
  })
);

const MyComponent = container({
  React,
  Button: button(React),
  Text: text(React),
}, buttonBehavior);

can be now improved to

const MyComponent = reactStamp.compose(
  React,
  Button: button(React),
  Text: text(React),
  buttonBehavior,
  {
    state: {},
    render(){}
  });

or even

const MyComponent = reactStamp
.compose(React)
.compose({
  Button: button,
  Text: text
})
.compose(buttonBehavior)
.compose({
  state: {},
  render(){}
});

Something like that. Up to you actually. :)

@troutowicz what do you think?

troutowicz commented 8 years ago

I think I need to play catch up with the spec! I'll come back to this once I do. :)

koresar commented 8 years ago

I'll ping you again few days later with implementation example and explanations. :)

troutowicz commented 8 years ago

Actually, after looking closer, I don't think either of your examples would work. React, Button, and Text are not stamps. In the container, React is passed to reactStamp returning a React stamp. Button and Text are stateless React components and get passed to container because they both get referenced in the render method of the HOC produced by the container.

koresar commented 8 years ago

Hint: https://github.com/stampit-org/stamp-specification/pull/71

troutowicz commented 8 years ago

Yeah, I'm not seeing it. Can you tell me what you expect the result of the below composition to be? I realize we are now supporting custom compose methods but I don't see how that could permit what you are proposing with the example.

const MyComponent = reactStamp.compose(
  React,
  Button: button(React),
  Text: text(React),
  buttonBehavior,
  {
    state: {},
    render(){}
  });

How could Button and/or Text get referenced inside the render method?

koresar commented 8 years ago

There is a typo of course. Here it corrected:

const MyComponent = reactStamp.compose(
  React,
  {
    Button: button,
    Text: text,
  },
  buttonBehavior,
  {
    state: {},
    render(){}
  });

There are few ways. The simplest one - React, Button, Text, state and render are all transformed to properties. So render can look like this:

render() {
  const {React, state, Button, Text} = this;
  return <Button><Text/></Button>; // something list this...
}

The Button and Text can reference the React the same way even though they are HOCs.

const React = {this};
troutowicz commented 8 years ago

OK, your typo corrections have made a world of difference. :) This gives me some ideas to play with, and has also convinced me that the BDC example in the README is way too convoluted.

koresar commented 8 years ago

too convoluted, mate!

troutowicz commented 8 years ago

@koresar see PR #32 and the bdc repo. After simplifying the BDC example, I'm not to sure your suggested improvements are needed anymore. Let me know what you think. :)

koresar commented 8 years ago

@troutowicz took a look at the BDC. Didn't understand. Can't find the simplification you're talking about.

troutowicz commented 8 years ago
const Component = reactStamp(React).compose(
  container,
  buttonBehavior
);

Container is no longer a factory, just a smart component. I'm not sure I'm understanding why you think the compose method needs an API change.

koresar commented 8 years ago

I wouldn't say it's "API change". I'd call it "additional API", more fluent one (probably).

troutowicz commented 8 years ago

Going to close this out for now.