grapp-dev / stacks

A set of components for building layouts in React Native. Powered by React Native Unistyles.
https://stacks.grapp.dev
MIT License
982 stars 24 forks source link

Stack doesn't play nice with Fragment #5

Closed gfpacheco closed 4 years ago

gfpacheco commented 4 years ago

If you have some complex conditionals in your render function you're likely to have something like this:

function MyComponent({ count }) {
  function renderChildren() {
    if (count === 1) {
      return <Text>One</Text>;
    }

    if (count === 2) {
      return <Text>Two</Text>;
    }

    return (
      <>
        <Text>Too</Text>
        <Text>Many</Text>
      </>
    );
  }

  return (
    <Stack space={1}>
      {renderChildren()}
    </Stack>
  )
}

But Stack doesn't work that way, it'll try to apply the spacing on the Fragment instead of its children.

Currently, the only way to make this work is replacing the Fragment with an array:

return [
  <Text key={0}>Too</Text>,
  <Text key={1}>Many</Text>,
];

Any thoughts? Maybe another workaround, ideally a fix

mobily commented 4 years ago

@gfpacheco thanks for reporting this problem, I'll take a look at it later today :)

mobily commented 4 years ago

@gfpacheco initially, I thought it's a bug but after further exploration, I'm aware that it isn't ;) according to the docs, Fragment is a component that let you group a list of children without adding extra nodes but under the hood Fragment is treated as a component that wraps other ones. At first, as a potential solution, I was thinking about extracting children from Fragment, but it would be redundant and I'd like to avoid it.

But let's get back to your problem :)

For single elements, you don't need to wrap them in the Stack component.

<Stack space={1}> 
  <Text>…</Text> // the last child's margin value is always 0
</Stack>

Therefore, your component can be written like this:

function MyComponent({ count }) {
  function renderChildren() {
    if (count === 1) {
      return <Text>One</Text>
    }

    if (count === 2) {
      return <Text>Two</Text>
    }

    return (
      <Stack space={1}>
        <Text>Too</Text>
        <Text>Many</Text>
      </Stack>
    )
  }

  return renderChildren()
}

Alternatively, the Stack component can be nested like this:

function MyComponent({ count }) {
  function renderChildren() {
    if (count === 1) {
      return <Text>One</Text>
    }

    if (count === 2) {
      return <Text>Two</Text>
    }

    return (
      <Stack space={1}>
        <Text>Too</Text>
        <Text>Many</Text>
      </Stack>
    )
  }

  return <Stack space={1}>{renderChildren()}</Stack>
}

Let me know if that makes sense. I'm also open to any suggestions :)

gfpacheco commented 4 years ago

Oh yes, I know it's not a bug! And I know how to work around it (of course my example was too simple).

I just thought that maybe that idea of extracting the children from Fragment would be a good one. But I'm not sure that it doesn't have other implications.

Thanks anyway, just wanted you to know and have two heads thinking about it.