Event Subscriptions not removed from fields that have conditional rendering applied

  • 1
  • Problem
  • Updated 3 years ago
  • Acknowledged
  • (Edited)
When a field in a field editor has conditional rendering applied, the eventSubscriptions are not being properly removed causing "render" to be called incorrectly.  This leads to several unintended consequences including performance hit, html being drawn then removed, etc.

Notes:
1) This one took several hours to track down and isolate for a small repro. When conditonallyDo gets called (as a result of models.loaded firing), data('rendered') is undefined and hence render gets called.  However, the field shouldn't be listening for models.loaded at this point.  Was very difficult to follow everything going on so I could likely be mistaken but it appears that the event subscriptions are being removed from the nx-field instead of the wrapperElement where they reside.
2) when render gets called, field.row is valid and contains data while field.model.data is empty
3) If you remove the conditional rendering, the problem does not occur.  

Steps to reproduce:
1) Create page using XML below
2) Preview page
3) dismiss "rendering!!!" alert when prompted
4) Click "ReQuery"

Expected Behavior
Render is not called and no alert is displayed

Actual Behavior
Render is called and alert is displayed

5) Refresh page
6) dismiss "rendering!!!" alert when prompted
7) Search for an account in the table search making sure to select an account that does is NOT the parent for any account (goal is to have the table return no results)

Expected Behavior
Render is not called and no alert is displayed

Actual Behavior
Render is called and alert is displayed

8) Remove the conditional rendering condition from Account Name
9) Repeat steps 2 thru 4

Expected = Actual - render is not called and no alert displayed

10) Repeat steps 5 thru 7 

Expected = Actual - render is not called and no alert displayed

Sample Page XML
<skuidpage unsavedchangeswarning="yes" personalizationmode="server" showsidebar="true" showheader="true" tabtooverride="Account">   <models>
      <model id="Account" limit="1" query="true" createrowifnonefound="false" sobject="Account" adapter="" type="" doclone="" processonclient="true">
         <fields>
            <field id="Name"/>
            <field id="CreatedDate"/>
            <field id="ParentId"/>
            <field id="Parent.Name"/>
            <field id="IsDeleted"/>
         </fields>
         <conditions>
            <condition type="fieldvalue" field="ParentId" operator="=" inactive="true" enclosevalueinquotes="true" name="__autofilter__ParentId" state="filterableoff" value=""/>
            <condition type="fieldvalue" value="" enclosevalueinquotes="true" field="Name" state="filterableoff" inactive="true" name="Name"/>
         </conditions>
         <actions/>
      </model>
      <model id="Tracker" limit="1" query="false" createrowifnonefound="true" adapter="" type="" sobject="Account" doclone="">
         <fields>
            <field id="DoNotShow" uionly="true" displaytype="BOOLEAN" label="DoNotShow"/>
         </fields>
         <conditions/>
         <actions/>
      </model>
   </models>
   <components>
      <pagetitle model="Account" uniqueid="sk-3K12SJ-70">
         <maintitle>
            <template>{{Name}}</template>
         </maintitle>
         <subtitle>
            <template>{{Model.label}}</template>
         </subtitle>
         <actions>
            <action type="multi" label="ReQuery">
               <actions>
                  <action type="setCondition" model="Account" condition="Name" value="blahblahblah"/>
                  <action type="requeryModels" fieldmodel="Tracker" field="DoNotShow" enclosevalueinquotes="true" value="true" behavior="standard">
                     <models>
                        <model>Account</model>
                     </models>
                  </action>
               </actions>
            </action>
         </actions>
      </pagetitle>
      <basicfieldeditor showheader="true" showsavecancel="false" showerrorsinline="true" model="Account" buttonposition="" uniqueid="sk-3KXS_s-88" mode="read">
         <columns>
            <column width="100%">
               <sections>
                  <section title="Section A" collapsible="no">
                     <fields>
                        <field id="Name" valuehalign="" type="CUSTOM" cssclass="" snippet="nameRenderer">
                           <renderconditions logictype="and" onhidedatabehavior="keep">
                              <rendercondition type="fieldvalue" operator="!=" enclosevalueinquotes="false" fieldmodel="Account" sourcetype="fieldvalue" field="IsDeleted" value="true"/>
                           </renderconditions>
                           <enableconditions/>
                        </field>
                     </fields>
                  </section>
               </sections>
            </column>
         </columns>
      </basicfieldeditor>
      <skootable showconditions="true" showsavecancel="false" showerrorsinline="true" searchmethod="server" searchbox="false" showexportbuttons="false" pagesize="10" createrecords="true" model="Account" buttonposition="" mode="read" uniqueid="sk-3K3_xl-526" emptysearchbehavior="query">
         <fields>
            <field id="Name" valuehalign="" type="" snippet=""/>
            <field id="CreatedDate"/>
            <field id="ParentId"/>
         </fields>
         <rowactions/>
         <massactions usefirstitemasdefault="true">
            <action type="massupdate"/>
            <action type="massdelete"/>
         </massactions>
         <views>
            <view type="standard"/>
         </views>
         <searchfields/>
         <filters>
            <filter type="select" filteroffoptionlabel="New Filter" createfilteroffoption="true" affectcookies="true" autocompthreshold="25" conditionsource="auto" labelmode="auto" conditionfield="ParentId"/>
         </filters>
      </skootable>
   </components>
   <resources>
      <labels/>
      <css/>
      <javascript>
         <jsitem location="inline" name="newInlineJS" cachelocation="false" url="">(function(skuid){
var $ = skuid.$;
    skuid.snippet.registerSnippet('nameRenderer', function(field, value) {
        var mode = field.mode
            , displayType = field.metadata.displaytype
            , renderer = skuid.ui.fieldRenderers[displayType];
        alert('rendering!!');            
        renderer[mode](field, value);
    });
})(skuid);</jsitem>
      </javascript>
   </resources>
   <styles>
      <styleitem type="background" bgtype="none"/>
   </styles>
</skuidpage>
Photo of Barry Schnell

Barry Schnell, Champion

  • 18,076 Points 10k badge 2x thumb

Posted 3 years ago

  • 1
Photo of Barry Schnell

Barry Schnell, Champion

  • 18,076 Points 10k badge 2x thumb
Noticed this block of code which seems very suspect.  Might be correct but the fact that render() is called and then immediately unregisterField is called seemed odd.

                   b.registeredFields && aa.each(b.registeredFields, function() {                        this.render && this.render(),
                        b.unregisterField(this)
                    }),

The block above is inside of the block of code that gets triggered after model retrieval

aa.when.all(n).done(function() { ... }
Photo of Barry Schnell

Barry Schnell, Champion

  • 18,076 Points 10k badge 2x thumb
lol, thanks Ben!
Photo of Ben Hubbard

Ben Hubbard, Employee

  • 12,490 Points 10k badge 2x thumb
I suspect that this is a remnant from skuids past and that removing the render there would fix everything. I'll just need to make sure this won't break other things.
Photo of Barry Schnell

Barry Schnell, Champion

  • 18,076 Points 10k badge 2x thumb
yeah, that makes sense.  Weird that registerItems destroys, registerdFields renders & unrenders, registeredLists renders and then after all that, handleDataRefresh gets called.
Photo of Tony Thomas

Tony Thomas, Employee

  • 80 Points 75 badge 2x thumb
This issue is addressed in Rockaway Update 1 which is available now at https://www.skuid.com/skuidreleases
Photo of Barry Schnell

Barry Schnell, Champion

  • 18,076 Points 10k badge 2x thumb
Tested and confirmed with Skuid 8.8.  Thank you!