MoroccanOSS / react-stoon

react-stoon is a handy toolbox of reusable Components that allow you to reduce boilerplate by writing less js inside of your React Components.
MIT License
29 stars 10 forks source link

Show can't render multiple elements #13

Open elhoucine opened 6 years ago

elhoucine commented 6 years ago

Hi @Amine-H I like the new release, the helpers look clean 👍

There is a little issue, Show supports only returning one element.

This will not work:

<Show if={true}>
  <ProjectItem />
  <p>Other element</p>
</Show>

React 16 supports this with Fragments, not sure if it will create compatibility issues with projects using older versions.

export default (props) => {
  const { if: visible, children } = props;
  if(visible){
    return <React.Fragment> {children} </React.Fragment>
  }

  return null;
}

Let me know if you have some solutions in mind.

Amine-H commented 6 years ago

Hey @elhoucine , thanks for your support!

Yeah, that would be an Issue with all of the Helpers, for the compatibility Issue, we'd check for the Existence of React.Fragment before using it, otherwise we'd just return {children}.

elhoucine commented 6 years ago

That’s good, I think we should also do something when Fragment does not exist and the component receives multiple elements, like using a div plus a warning about it or simply state the limitation in the docs. The div tradeoff seems better to me.

Amine-H commented 6 years ago

react-stoon shouldn't use any component specific to a ReactDOM, ReactNatvie or ReactVR. also, there's not much we could for the case when children is an array and React.Fragment isn't there, the user must be aware of the limitations.

elhoucine commented 6 years ago

Alright it makes sense.

yjose commented 6 years ago

Hi friends, I think the show component will return multiple components correctly and out of the box using react 16 without adding the fragment element. check this example https://codesandbox.io/s/qqxkxx9oo9. But I think the best way to handle complex use case is to use the react.Children API https://reactjs.org/docs/react-api.html#reactchildren . @Amine-H I have a problem when I import the show component form the npm repo

Invariant Violation
Element type is invalid: expected a string (for built-in components) or a class/function (for composite components) but got: undefined. You likely forgot to export your component from the file it's defined in, or you might have mixed up default and named imports.

Thanks

Amine-H commented 6 years ago

Hello, thanks @yjose , I was going to test it, in anyway I use the React.Children API whenever needed, good news that Show works out of the box.

as for the npm repo, I deprecated @moroccan-oss/react-stoon in favor of react-stoon https://www.npmjs.com/package/react-stoon

sorry for any inconvenience that this might have caused.