kobotoolbox / kpi / 27608652545

82%

master: 76%

push

github

fix(editableForm): stale closure and cache mutability issues DEV-2260 (#7139)

### 📣 Summary

Fixed two bugs in Form Builder: event listeners weren't cleaning up on
unmount (memory leak), and the react-query cache was exposed to direct
mutation from Backbone code.

### 💭 Notes

Changes here:

**jsapp/js/editorMixins/EditableForm.tsx**
- Added `appRef` to hold the SurveyApp instance
- The cleanup function was closing over `app=undefined` from the initial
render, so all the `.off()` calls were no-ops on unmount
- The ref gives cleanup access to whatever app instance is actually
alive when it runs
- Deep clone the asset from react-query before storing it in state
- The old stores.allAssets code had an explicit warning comment about
this: "Form Builder code is already changing the content of the object"
- After the refactor, we were handing the cache reference straight to
Backbone, which meant any push/splice on `survey.availableFiles` would
corrupt the cache
  - Restored the clone that was there before
- Updated cleanup to read from the ref
- Store the app in both state and ref when creating it

Joke from AI: The cleanup function was like trying to turn off lights in
a room after the door's been closed - it couldn't see in anymore. The
ref is the window.

### 👀 Preview steps

The bugs are not really testable/reproducible. We are making the code
more robust (to avoid hard to debug bugs in future) based on analysis
from greptile and verifying its findings by looking at the code.

9034 of 12268 branches covered (73.64%)

29662 of 36357 relevant lines covered (81.59%)

5.56 hits per line