30 October 2017

Written by Jon Bevan

This can't be happening

When your colleague tells you that the UI in your product is no longer saving stuff, that's a low point in the day.

The save button on several pages in our Jira Cloud add-on appeared to do absolutely nothing.

That shouldn't happen

We quickly checked the Developer Tools in our browser for (new) errors. There were none.

A few console log statements later and we realized that the save button does some web form validation and it was erroneously reporting that the form contents were invalid. A few more log statements later and sure enough, the data being validated was incorrect, but the form's contents didn't match the data that was being validated. The data being validated was an old copy of the form data...

Why does that happen?

We scratched our heads. That makes no sense.

I then remembered something about functions being closures and variables (or consts, because who doesn't love immutability) being closed over etc etc... so we looked at the event handier code on the button:

const SecondaryButtons = ({title, onSave, onCancel, saving }) => {
    const { inProgress } = saving;
    return (
        <div className='saveCancelRow'>
            <ButtonsWrapper>
                <Button
                    handleClick={onSave}
                    title={title}
                    disabled={inProgress}>Save</Button>
                <Button
                    disabled={inProgress}
                    title='Cancel changes'
                    handleClick={(e) => {
                        e.preventDefault();
                        onCancel();
                    }}>Revert</Button>
            </ButtonsWrapper>
        </div>
    );
};

// ... code omitted for brevity

const PageForm = connect(mapStateToProps)(({ formData, saving, dispatch }) => {
    return (
        <Form>
            <SecondaryButtons
                title='Save page'
                saving={saving}
                onCancel={() => {
                    dispatch(revertChangeAction(formData.uuid));
                }}
                onSave={() => {
                    dispatch(validateAndSave(formData));
                }} />
        </Form>
    );
});

We quickly changed the code so we pass the form contents (formData) around to make sure we close over the right value. New console logs added to the SecondaryButtons component to ensure we get the right data in the component.

const SecondaryButtons = ({title, onSave, onCancel, saving, formData }) => {
    console.log('DEBUG formData', formData);
    const { inProgress } = saving;
    return (
        <div className='saveCancelRow'>
            <ButtonsWrapper>
                <Button
                    handleClick={() => {
                        console.log('DEBUG handleClick', formData);
                        onSave(formData);
                    }}
                    title={title}
                    disabled={inProgress}>Save</Button>
                <Button
                    disabled={inProgress}
                    title='Cancel changes'
                    handleClick={(e) => {
                        e.preventDefault();
                        onCancel(formData.uuid);
                    }}>Revert</Button>
            </ButtonsWrapper>
        </div>
    );
};

// ... code omitted for brevity

const PageForm = connect(mapStateToProps)(({ formData, saving, dispatch }) => {
    return (
        <Form>
            <SecondaryButtons
                formData={formData}
                title='Save page'
                saving={saving}
                onCancel={(uuid) => {
                    dispatch(revertChangeAction(uuid));
                }}
                onSave={(data) => {
                    dispatch(validateAndSave(data));
                }} />
        </Form>
    );
});

Same behaviour. Form is marked as invalid and refuses to submit because the wrong data is being validated.

We add an additional log statement just to make sure that the data the component receives is what is used in the event handler.

It isn't...

Wat? We literally just logged that the component gets given the right data. And somehow the old data is used when validating?! It's like the event handler hasn't been updated...

How could that even happen? We use React and we know the component is being rendered because of our log statement. Right?

Nope.

"Internally, React uses several clever techniques to minimize the number of costly DOM operations required to update the UI" - https://reactjs.org/docs/optimizing-performance.html

Oh, I see

There's more at https://reactjs.org/docs/reconciliation.html

OK... so maybe one of the main reasons for using React has come back to bite us. React only re-renders the UI when it changes...

In our case, the UI didn't change - the button still says 'Save' each time the form data changes. So only changing the event handler is not enough to trigger a re-render, because although our component's render function was being executed, it wasn't returning anything different to what React knew was already rendered...

How did that ever work?!

It didn't... So what's the solution?

An ugly hack is to use a key prop based off the form data contents. At least, that's what we did to get a bug fix deployed. In the long term we should decouple the validation/saving logic so it pulls the data from our Redux store on demand instead of relying on the props given to the component at render time.

Remember kids, don't expect React to re-render your components if there are no visible changes to them DOM output!



blog comments powered by Disqus