ctrlplusb / easy-peasy

Vegetarian friendly state for React
https://easy-peasy.dev
MIT License
5.03k stars 190 forks source link

Possible bug due to unnecessary rerenders #902

Closed NilsBaumgartner1994 closed 7 months ago

NilsBaumgartner1994 commented 7 months ago

I don't get it, why in this simple example the components MyButtonRowWithOptionKey are rerendering both, despite i only change a value in one?

import {Text, View} from 'react-native';
import {useCallback} from "react";
import {action, createStore, StoreProvider, thunk, useStoreActions, useStoreState} from "easy-peasy";
import {TouchableOpacity} from "react-native";

const store = createStore({
    options: {},
    updateOptions: action((state, payload) => {
        state.options = payload;
    }),
    setOptions: thunk(async (actions, payload, { getState }) => {
        let currentState = getState().options;
        const newValue = await payload(currentState)
        actions.updateOptions(newValue);
    }),
});

function useFlipOptions() {
//  const [options, setOptions] = useState<Record<string, boolean>>({});
    const options = useStoreState((state) => state.options);
    const setOptions = useStoreActions((actions) => actions.setOptions);

    const flipOptions = useCallback((option: string) => {
        setOptions((currentOptions) => {
            let newOptions = currentOptions ? {...currentOptions} : {};
            newOptions[option] = !newOptions[option];
            return newOptions;
        })
    }, [])
    return {options, flipOptions}
}

function useFlipOptionsWithKey(optionKey: string) {
    const {options, flipOptions} = useFlipOptions();

    const specificFlipOption = useCallback(() => flipOptions(optionKey), [optionKey])

    return {optionValue: options?.[optionKey], specificFlipOption}
}

function MyButtonRowWithOptionKey({optionKey}: {optionKey: string}) {
    const {optionValue, specificFlipOption} = useFlipOptionsWithKey(optionKey);

    console.log(`Rendering MyButtonRowWithOptionKey ${optionKey}`)

    return (
        <View>
            <Text>{optionKey+": "+optionValue}</Text>
            <TouchableOpacity style={{
                backgroundColor: 'orange',
                padding: 10,
                margin: 10,
            }} onPress={specificFlipOption}>
                <Text>Flip {optionKey}</Text>
            </TouchableOpacity>
        </View>
    )
}

export default function PlaygroundTestScreen() {

    return (
            <StoreProvider store={store}>
            <View style={{
                flex: 1,
                height: '100%',
                width: '100%',
            }}
            >
                <Text>Options:</Text>
                <MyButtonRowWithOptionKey optionKey={'option1'} />
                <MyButtonRowWithOptionKey optionKey={'option2'} />
            </View>
            </StoreProvider>
    );
}
NilsBaumgartner1994 commented 7 months ago

one solution is to memorize the component with the optionValue. My initial though was that i could get the same instance of the methods back?

// memoize this component
function MyButtonRowWithOptionKey ({optionKey}: {optionKey: string}) {
    const {optionValue, specificFlipOption} = useFlipOptionsWithKey(optionKey);

    return useMemo(() => {
        console.log(`Rendering MyButtonRowWithOptionKey ${optionKey}`)

        return (
            <View>
                <Text>{optionKey+": "+optionValue}</Text>
                <TouchableOpacity style={{
                    backgroundColor: 'orange',
                    padding: 10,
                    margin: 10,
                }} onPress={specificFlipOption}>
                    <Text>Flip {optionKey}</Text>
                </TouchableOpacity>
            </View>
        )
    }, [optionValue])
}
jmyrland commented 7 months ago

Hey @NilsBaumgartner1994 !

I don't get it, why in this simple example the components MyButtonRowWithOptionKey are rerendering both, despite i only change a value in one?

Both components are re-rendered in both cases because both are dependent on the options object in the store.

When updateOptions is called, the options reference is updated (to the new object), which in turn triggers a rerender for all components depending on options from the store.

If you want to "fix" this in the store, you'd have to split the options into separate values in the store & update these individually.

Hope that clarifies things.

one solution is to memorize the component with the optionValue

Given that the options values are primitives, your example should prevent view-rerenders for unchanged options values.

NilsBaumgartner1994 commented 7 months ago

I found where the problem was:

The rerender was caused due to the option reference being updated. So as you said it triggers a rerender for all components depending on that.

If you want to "fix" this in the store, you'd have to split the options into separate values in the store & update these individually.

My solution would be this. Seems a bit easier in my optionion?

// BAD
const options = useStoreState((state) => state.options);
const optionValue = options?.[optionKey];

// Correct
const optionValue = useStoreState((state) => state.options?.[optionKey]);
NilsBaumgartner1994 commented 7 months ago

@jmyrland is my solution overlooking something?

jmyrland commented 7 months ago

I found where the problem was:

The rerender was caused due to the option reference being updated. So as you said it triggers a rerender for all components depending on that.

If you want to "fix" this in the store, you'd have to split the options into separate values in the store & update these individually.

My solution would be this. Seems a bit easier in my optionion?

// BAD
const options = useStoreState((state) => state.options);
const optionValue = options?.[optionKey];

// Correct
const optionValue = useStoreState((state) => state.options?.[optionKey]);

I think that should work. It should only rerender when the options[optionKey]-state changes 👍

My only concern is; what happens if options?.[optionKey] resolves to undefined. If you have tested this, and it works - you should be fine 👌