equinor / mad

Experimental monorepo for the Mobile Application Delivery team
https://equinor.github.io/mad/
MIT License
8 stars 2 forks source link

Updating the state of a cell causes all cells in cellgroup to re render #469

Open HavardNot opened 8 months ago

HavardNot commented 8 months ago

Package / App

MAD Components

Version

0.13.0

What happened?

Expected behaviour: When the state of a cell is updated, only the current cell should be re-rendered Actual behaviour: All cells in the cell group are re-rendered.

When working with large cell groups this causes slowness in the app. This can probably be fixed by using memoization

Other info

No response

Relevant log output

No response

HaakonSvane commented 7 months ago

@HavardNot I tried recreating this by creating a simple test screen:

const StateCell = () => {
    const renderCounter = useRef(0);
    renderCounter.current += 1;
    const [myState, setMyState] = useState<number>(0);

    return (
        <Cell style={{}}>
            <Typography>Rendered {renderCounter.current} times</Typography>
            <Button title="Cause rerender" onPress={() => setMyState(state => state + 1)} />
        </Cell>
    );
};

export const TestScreen = () => {
    const style = useStyles(themeStyles);
    return (
        <ScrollView
            contentInsetAdjustmentBehavior="automatic"
            contentContainerStyle={style.contentContainer}
        >
            <Cell.Group>
                <StateCell />
                <StateCell />
                <StateCell />
            </Cell.Group>
        </ScrollView>
    );
};

const themeStyles = EDSStyleSheet.create(theme => ({
    contentContainer: {
        paddingVertical: theme.spacing.container.paddingVertical,
    },
}));

And got the following results:

https://github.com/equinor/mad/assets/36899138/0cd770cf-52f9-42a8-80a2-6751076c9f7b

It seems that state changes inside single cells do not cause others to rerender. Was this what you meant, or was the issue you described triggered by something else?

HavardNot commented 6 months ago

@HaakonSvane I was having this issue using it for Country Information, but I was struggling to reproduce the issue in Chronicles. @tormodAase did you take a look at this, and did you manage to reproduce the problem?

tormodAase commented 6 months ago

Nope, I can try to reproduce the issue this week