Kelin2025 / effector-factorio

The simplest way to write re-usable features with React + Effector
MIT License
62 stars 6 forks source link

Why `model` prop is passed to original view component? #8

Open 7iomka opened 1 year ago

7iomka commented 1 year ago

Hi. Why model prop is passed to original view component? This is needed only for types? Because I wasn't expected to receive it alongside with my component props.

const FeatureComponentSelect = modelView(factory, (props: SelectProps) => { const model = factory.useModel(); // some code inside return <Select {...props} /> })

So in the DOM I have model prop passed. So this is a problem, and I need somehow to make 2 steps to solve this issue 1) every type of component wrapper I need to mix to something like this props: SelectProps & { model: ReturnType<typeof factory.useModel> } 2) if component passed rest props with spread, every time you need to extract model prop from it

So, my question is - why you pass model prop to original component if you never use it (model is always is extracted through factory.useModel hook)

My suggestion is to separate viewProps from model like this

 const { model, ...viewProps } = props;
mistical2008 commented 1 year ago

It is useful in cases where you don't have children components but you want to create reusable feature. Created view has "model" field in it's props signature. Reading types is better than expecting.

SQReder commented 10 months ago

99% of components in our projects use model directly from props And just like 2-3 times I was needed to use useModel hook to get model in child components

7iomka commented 10 months ago

99% of components in our projects use model directly from props And just like 2-3 times I was needed to use useModel hook to get model in child components

This means that you need to add type of model in component props in 99% cases?

SQReder commented 8 months ago

99% of components in our projects use model directly from props And just like 2-3 times I was needed to use useModel hook to get model in child components

This means that you need to add type of model in component props in 99% cases?

I think it's kinda cheap

type Props = {
    model: Model<typeof myCoolModelFactory>
    otherProp: unknown
}

const MyCoolView = modelView(myCoolModelFactory, ({model, otherProp}: Props) =>{
    return null;
})

Is you haven't any extra props, so better

const MyCoolView = modelView(myCoolModelFactory, ({model}) =>{
    return null;
})