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!
- 09 Oct 2018 » A strange bug on AWS Lambda
- 17 Jan 2018 » How to run Karma tests in browsers in Docker
- 07 Dec 2017 » Switching from Javascript to Typescript
- 30 Oct 2017 » Fun with React event handlers
- 17 Jul 2017 » Switching from Groovy to Java
- 24 May 2017 » Useful Git Aliases
- 27 Mar 2017 » Practical Ratpack Promises
- 03 Nov 2016 » Custom Content in Forms for Confluence Connect
- 04 Oct 2016 » Checking user permissions from REST calls
- 30 Sep 2016 » Using the reflection API in Confluence
- 28 Sep 2016 » Creating a custom Confluence Blueprint
- 06 Sep 2016 » ReactJS in Forms for Confluence Connect
- 25 Apr 2016 » Migrating to ES6 in Atlassian Add-ons
- 17 Mar 2016 » All kinds of things I learnt trying to performance test against Fisheye/Crucible
- 24 Dec 2015 » Adaptavist’s Holiday Gift of Atlassian Deployment Automation
- 17 Dec 2015 » Getting a Custom Field value safely
- 07 Dec 2015 » Putting Google Analytics to work with plugins for Confluence
- 02 Dec 2015 » Devoxx Voting, A retrospective
- 25 Nov 2015 » Some things I've learnt about SingleSelect
- 15 Oct 2015 » Using SOY for JIRA actions
- 26 Sep 2015 » Object Reflection in Groovy
- 22 Sep 2015 » Introducing Adaptavist Labs