Metadata
- Source
- FLUID-5282
- Type
- Bug
- Priority
- Major
- Status
- Closed
- Resolution
- Fixed
- Assignee
- Cindy Li
- Reporter
- Antranig Basman
- Created
2014-03-04T05:13:04.232-0500 - Updated
2014-03-25T10:11:07.811-0400 - Versions
- N/A
- Fixed Versions
-
- 1.5
- Component
-
- Data Binder
- Renderer
Description
FLUID-3674 introduced major changes to the model semantic for the ChangeApplier. The scope of these is described at http://wiki.fluidproject.org/display/fluid/New+Notes+on+the+ChangeApplier . Unfortunately, one of the clients of the old ChangeApplier that got caught out is our own RendererUtilities for working on the so-called "protoComponents". Although we plan to remove all of this renderer architecture wholesale, we need to keep it working in the meantime. cindyli ran into this bug whilst working on the metadata component, original test case supplied at https://github.com/amb26/infusion/pull/3/files . This issue can be demonstrated a good deal more simply than this, and is caused by the fact that the entire protocomponent expansion pathway closes over the reference to "model" at the outset and will therefore ignore all changes supplied via the new ChangeApplier which repeatedly rebind this reference at the end of each transaction.
From RendererUtilities.js line 535:
var expandConfig = {
model: options.model,
This evil runs deep! Rather than pollute all of this code with references to "holder" etc. a quick enough hack should be to manually rebind to the model whenever we receive a "refreshView" since is the only lifecycle point at which protocomponent work should be occurring. This fix to this doomed architecture only needs to be "good enough" ...
Both "options" and "expandConfig" -> "config" are sources of this reference pollution
Comments
-
Colin Clark commented
2014-03-18T20:51:57.436-0400 I have reviewed, tested, and pushed this fix to the project repo's master branch at revision 527d757fe8a1020a9356416e49791c7c925b8b3d.
Cindy Li can you confirm that this fix resolves the issue and close it if so?
-
Cindy Li commented
2014-03-25T10:11:07.786-0400 Confirmed the merged fix does resolve the issue.