FlatList props

[fortmarek]

Currently, we define FlashList props like this:

export interface FlashListProps<T> extends FlatListProps<T>

This means we implicitly allow usage of all FlatListProps - regardless of whether we support them or not. I find this concerning - users will use props they know but they won't do anything. We also reuse the documentation of those props from FlatList - but we might want to change it to better suit our list.

I propose to define all the FlatList explicitly:

export interface FlashListProps<T> extends FlatListProps<T> {
   /**
   * Rendered in between each item, but not at the top or bottom
   */
  ItemSeparatorComponent?: FlatListProps<ItemT>["ItemSeparatorComponent"];
  // or to ensure the typing is the same (please, let me know which version you prefer):
  ItemSeparatorComponent?: React.ComponentType<any> | null | undefined;
   ...
}

Cons:

• we can diverge from FlatList without us knowing so. The biggest issue would be if FlatList renames a prop but I don't see that happening too often.
• we will have to watch closely new additions - but that's the case even with current approach
• increased maintenance burden since we need to define those props ourselves

Pros:

• users can't define props we actually don't support. Imho, this is highly important and makes worth making a change even with the cons above
• we have control over the documentation of the props, so we can freely edit it to better fit FlashList behavior

What do you think? Should we make this change? 👀

[naqvitalha]

I support this idea. It makes sense and I also think if we see good adoption in community then diverging might also make sense because we can offer better APIs compared to FlatList. Not going to change much right now but it's definitely worth doing once FlashList becomes the default RN list (big hopes) :P

What timeline did you have in mind for this?

[fortmarek]

because we can offer better APIs compared to FlatList

True! It does make us more flexible altogether in the long run 🏃

What timeline did you have in mind for this?

I think this should be done before opensourcing but wouldn't say it's on the top of the list in terms of priorities.

wdyt @ElviraBurchik @davebcn87? If everybody agrees, I can add it as an issue to the opensource milestone.

[naqvitalha]

If we do it before open sourcing we need to ensure type compatibility to make sure drop in replacement is as seamless as possible.
If type errors etc show up then it'll not be ideal. After people know about how good it is it'll be really easy to start changing things because people will cooperate.

[ElviraBurchik]

My opinion is it looks contradicting our main selling point: the same API as FlatList and on the fly migration. With this change we basically start deviating - we don't depend on FlatList props any more and gonna slowly move further from the initial idea of a faster list with the same API. IMHO, if it's still the same concept we follow we need to support as much props as possible and notify which are not available or admit that it's no longer our goal to be as similar as FlatList as possible.

[fortmarek]

We can still use ItemSeparatorComponent?: FlatListProps<ItemT>["ItemSeparatorComponent"]; version to ensure the typing for the props we do support is the same.

But there are props that we never plan to support like getItemLayout and we currently allow specifying those props. This will just confuse users. I am not advocating for starting to deviate from FlatList but to use type system to help users navigate which props we support or not.

we need to support as much props as possible and notify which are not available

See, this is exactly what I want this change to achieve - properly notify which props are not available. How else would you do that? Throw an exception? That is possible but not pretty. The proposed solution uncovers that issue during compilation which is the ideal behavior imho.

[ElviraBurchik]

I see what you mean, that makes sense 👍

[fortmarek]

Created an issue here

[davebcn87]

Isn't this more close to what we had in the beginning? When I started the wrapper I manually created every prop for that reason, only the implemented ones should be there.

I like this option, to make sure the parameters are exactly the same, but we will need to be careful every time FlatList changes one of the props, otherwise this could break our implementation:

ItemSeparatorComponent?: FlatListProps<ItemT>["ItemSeparatorComponent"];

[fortmarek]

Isn't this more close to what we had in the beginning? When I started the wrapper I manually created every prop for that reason, only the implemented ones should be there.

I am not really sure what we had in the beginning at this point to be quite frank 😅 but doesn't mean we can't return to it if that's the case.

I like this option, to make sure the parameters are exactly the same, but we will need to be careful every time FlatList changes one of the props, otherwise this could break our implementation

That is true - I think it actually might be a bad idea since the FlatList version is outside of our control. We should probably define the types explicitly.

[davebcn87]

I am not really sure what we had in the beginning at this point to be quite frank 😅 but doesn't mean we can't return to it if that's the case.

Yes, I'm mentioning that because we changed it for a reason. I think it was because there were type mismatches against FlatList props. I think this change was done by @naqvitalha and he could provide more context about the decision.