CellRendererComponent prop

[fortmarek]

Description

Fixes #360

We currently don't support CelllRendererComponent. It can be used for various use-cases, such as reanimated support.

In VirtualizedList, the component (if available) is used to wrap the cells (see here).

However, in FlashList we need the wrapping view to be CellContainer. There are multiple solutions, neither of them ideal:

• CellContainer can wrap CellRendererComponent -> most of the use-cases for CellRendererComponent leverage that the components are at the same level, so this won't work (e.g. layout animations don't trigger this way). This way, there's not much difference between moving the view from CellRendererComponent to renderItem
• CellRendererComponent can wrap CellContainer -> this will break AutoLayoutView since it assumes CellContainer is on the top level. The adjustments would also be done on CellContainer and not CellRendererComponent leading to additional bugs
• CellRendererComponent must return a component that extends CellContainer. This solution does work well but returning an element extending CellContainer constraints the usage. For example, for reanimated support, you need to return component created with Animated.createAnimatedComponent(CellContainer). This is not going to be of type CellContainer

As mentioned, all of these options have flaws. The first option will bring us the closest to FlatList if we are able to solve the issues of finding CellContainer in the view hierarchy and then adjusting CellRendererComponent instead of CellContainer. The last option is the easiest to implement (as showcased by this PR) but afaict, brings a lot of limitations.

Reviewers’ hat-rack 🎩

• [ ]

Screenshots or videos (if needed)

Checklist

• I have added a changelog entry following the Keep a Changelog guidelines.

[naqvitalha]

We only need CellComponentRenderer to render a CellContainer as the first child. A custom component is also fine as long as it returns CellContainer. Since it's just a view I don't see any limitations. We just need to tweak our type.

As part of this PR we should also define the type of props that are passed to CellContainer which will make it easier for the community to tweak it and create awesome stuff. e.g, data, x, y, height, width, extendedState

[fortmarek]

We only need CellComponentRenderer to render a CellContainer as the first child. A custom component is also fine as long as it returns CellContainer. Since it's just a view I don't see any limitations. We just need to tweak our type.

This would have been fine if it was not necessary for the custom view to be the first child - but it is for reanimated and I presume for other use-cases:

// This does not work
const createCellRenderer = () => {
  const cellRenderer: React.FC = (props) => {
    return (
      <CellContainer>
        <Animated.View layout={Layout.duration(3000)}>
          {props.children}
        </Animated.View>
      </CellContainer>
    );
  };

  return cellRenderer;
};

// This works
const createCellRenderer = () => {
  const cellRenderer: React.FC = (props) => {
    return (
      <Animated.View layout={Layout.duration(3000)}>
        <CellContainer>{props.children}</CellContainer>
      </Animated.View>
    );
  };

  return cellRenderer;
};

...
CellRendererComponent={cellRenderer}
...

I have not tried out other use-cases but it's plausible they will have the same limitation - if that's the case, we hit the issues outlined originally in the PR description. If we could return CellContainer as the first child, it would have been definitely made things easier, but it seems we cannot without sacrificing the utility of CellRendererComponent prop in the first place.

[naqvitalha]

Since CellContainer is a native component with a view backing it. AnimatedCellContainer should easy to build. E.g, createAnimatedComponent(CellContainer)
It should work I think.

[fortmarek]

E.g. createAnimatedComponent(CellContainer)

You're right, that does work. To make TS satisfied, I had to change the type here to React.ComponentType<any> | undefined. typeof CellContainer resolves to HostComponent<unknown> anyway, so there's no bulletproof way really to check statically whether users actually pass CellContainer inside CellRendererComponent.

If we are ok with abandoning TS checks for the CellRendererComponent, we could still probably check dynamically in AutoLayoutView whether its subviews are of the expected type CellContainer.

Wdyt, @naqvitalha?

[davebcn87]

You're right, that does work. To make TS satisfied, I had to change the type here to React.ComponentType | undefined. typeof CellContainer resolves to HostComponent anyway, so there's no bulletproof way really to check statically whether users actually pass CellContainer inside CellRendererComponent.

Worst case scenario, if they don't add the CellContainer AutoLayoutView won't work and they will see some glitches while rendering / rotating the device, right?

[davebcn87]

I'm ok with the dynamic check, if possible. This could throw a warning to the devs.

[fortmarek]

Worst case scenario, if they don't add the CellContainer AutoLayoutView won't work and they will see some glitches while rendering / rotating the device, right?

Yes, that's correct.

[naqvitalha]

@fortmarek There is no way to type JSX right? If there isn't then I guess we can just call it out in comments that return type should be CellContainer.

[fortmarek]

There is no way to type JSX right? If there isn't then I guess we can just call it out in comments that return type should be CellContainer.

Yes, that's correct.

I think this is ready for review now. If the root native component in the renderer native view hierarchy is not CellContainer, we will throw an error from the native side.

It still might not allow all use-cases since there might be cases where you don't want the root component of CellRendererComponent to be CellContainer. But at least should unblock us for reanimated (see example in usage).

[ElviraBurchik]

The changes looks good in theory. Is there any way to easily test it in fixture?

[fortmarek]

The changes looks good in theory. Is there any way to easily test it in fixture?

Just to check if things render correctly, you can follow this example.

Or you can use this commit for layout animation - it won't work 100 % correctly since it clashes with recycling - but it's not something we want to perfect on our side. We're good with this being possible in theory.

Eventually, we could add a more sophisticated example (e.g. using the draggable functionality that Shopify Mobile aims to build)

[fortmarek]

@naqvitalha please, have another look at this 🙏

[naqvitalha]

We can also add some documentation to CellContainer mentioning that it's just a View and can be used with createAnimatedComponent. Mentioning some of its useful props can also be beneficial like x, y, height, width, data

[fortmarek]

Added: bfa8fe7

[naqvitalha]

This looks good. It'll be great to have some more details about CellContainer added

[davebcn87]

Eventually, we could add a more sophisticated example (e.g. using the draggable functionality that Shopify Mobile aims to build)

I like the idea

[naqvitalha]

Let's remove logs from here

[naqvitalha]

Let's mention index prop which is important for accessing data and remove logs from library code.