Skip to content

Conversation

TalmizAhmed
Copy link

@TalmizAhmed TalmizAhmed commented Oct 7, 2025

ImportData Repeatable Panel Bug - Investigation & Fix

Problem Statement

When using importData() with data containing multiple instances of a repeatable panel:

  • Expected: Display all instances from imported data (e.g., 2 instances if data has 2 items)
  • Actual: Only initial instances shown (e.g., 1 if minOccur=1), even though data model is correct
myForm.importData({
    "panel": [
        { "textinput": "French", "radiobutton": "1" },
        { "textinput": "Latin", "radiobutton": "0" }
    ]
});
// Before Fix: Only 1 instance visible, data corrupted ❌
// After Fix: 2 instances visible with correct data ✅

Root Cause & Solution

The bug had three interconnected issues, each requiring a specific fix:

Issue 1: Missing 'items' Notifications (Backend)

Problem: Form.importData()syncDataAndFormModel() added instances but never sent 'items' notifications.
Why: Only Container.importData() manually sent notifications after calling syncDataAndFormModel().
Impact: DOM never updated because Worker never notified main thread about new instances.

Solution: Make syncDataAndFormModel() ALWAYS send notifications when instances are added/removed.

// Modified: blocks/form/rules/model/afb-runtime.js (lines 2520-2530)
syncDataAndFormModel(contextualDataModel) {
    const result = { added: [], removed: [] };
    // ... add/remove instances ...
    
    // NEW: Always notify when instances change
    result.added.forEach((item) => {
        this.notifyDependents(propertyChange('items', item.getState(), null));
        item.dispatch(new Initialize());
    });
    result.removed.forEach((item) => {
        this.notifyDependents(propertyChange('items', null, item.getState()));
    });
    return result;
}

Result: Both Form.importData() and Container.importData() automatically get notifications. Simplified Container.importData() by removing duplicate notification code.

Issue 2: InstanceManager Has No DOM Element (Frontend)

Problem: InstanceManager (ID: panel) has no DOM element. Only instances exist: panel[0], panel[1].
Why: fieldChanged() tried to find #panel → not found → exited before handling 'items' notification.
Impact: Even with notifications, new instances never rendered.

Solution: Handle 'items' changes BEFORE checking if field exists. Find repeat-wrapper via first instance.

// Modified: blocks/form/rules/index.js (lines 75-104)
async function fieldChanged(payload, form, generateFormRendition) {
  const { changes, field: fieldModel } = payload;
  const { id } = fieldModel;
  const field = form.querySelector(`#${id}`);
  
  if (!field) {
    // NEW: Handle 'items' changes for InstanceManager
    const itemsChange = changes.find(c => c.propertyName === 'items');
    if (itemsChange) {
      const firstInstance = form.querySelector(`#${CSS.escape(id)}\\[0\\]`);
      const repeatWrapper = firstInstance?.closest('.repeat-wrapper');
      if (repeatWrapper) {
        const { currentValue, prevValue } = itemsChange;
        if (currentValue === null) {
          repeatWrapper.querySelector(`#${CSS.escape(prevValue.id)}`)?.remove();
        } else {
          const promise = generateFormRendition({ items: [currentValue] }, repeatWrapper);
          renderPromises[currentValue?.qualifiedName] = promise; // Store for sync
        }
        return;
      }
    }
    // ... existing retry logic ...
    return;
  }
  // ... handle other changes ...
}

Result: New instances render correctly. Render promise stored for synchronization with value changes.

Issue 3: Race Condition with Name Updates (Radio/Checkbox)

Problem: Radio/checkbox values set BEFORE names updated, causing interference.
Why: repeat.js updates names in requestAnimationFrame (async) for radio & checbox groups to have different names (so that their selection does not collide with other instances), but value changes processed immediately before the name property was updated, which meant the old bug of cross selection re-appeared.
Impact: Radio buttons with same name are mutually exclusive → selecting one deselects the other → data corruption.

Timeline (Broken):

T0: 'items' change → generateFormRendition starts (async)
T1: 'value' change → processes immediately with default names
    Both instances have name="radio" → radios interfere!
T2: generateFormRendition completes
T3: requestAnimationFrame → updates names (TOO LATE!)

Note: Doc-based forms don't have this issue (they update names synchronously). Only affects AF-based importData.

Solution: Wait for pending renders + requestAnimationFrame BEFORE processing radio/checkbox values.

// Modified: blocks/form/rules/index.js (lines 108-130)
async function fieldChanged(payload, form, generateFormRendition) {
  // ... 
  const field = form.querySelector(`#${id}`);
  if (!field) { /* ... handle 'items' ... */ return; }
  
  const fieldWrapper = field?.closest('.field-wrapper');
  
  // NEW: For radio/checkbox, wait for pending renders before processing values
  const hasValueChange = changes.some(c => c.propertyName === 'value');
  if (hasValueChange && (fieldType === 'radio-group' || fieldType === 'checkbox-group')) {
    // Extract container name: "$form.panel[0].radio" → "$form.panel"
    const containerMatch = qualifiedName?.match(/^(.*?)\[\d+\]/);
    const containerName = containerMatch ? containerMatch[1] : null;
    
    // Check if container is being rendered
    const matchingKey = Object.keys(renderPromises).find(key => 
      containerName && key.startsWith(containerName + '[')
    );
    
    if (matchingKey) {
      await renderPromises[matchingKey];                    // Wait for render
      await new Promise(resolve => requestAnimationFrame(resolve)); // Wait for name updates
      delete renderPromises[matchingKey];
      await fieldChanged(payload, form, generateFormRendition); // Retry with correct names
      return;
    }
  }
  
  // Process changes (now names are correct)
  changes.forEach(change => { /* ... */ });
}

Timeline (Fixed):

T0: 'items' change → generateFormRendition starts, stores promise
T1: 'value' change → detects pending render → WAITS
T2: generateFormRendition completes
T3: requestAnimationFrame → updates names
T4: Retry value change → processes with correct names ✅

Result: Value changes wait for name updates. Radio buttons in different instances have unique names, preventing interference.

Complete Flow (Fixed)

myForm.importData({ panel: [data1, data2] })
  ↓
syncDataAndFormModel() adds panel[1]
  ↓ [NEW]
notifyDependents(propertyChange('items', panel[1]))
  ↓
Worker → Main Thread: 'items' change
  ↓
fieldChanged() detects 'items', no field found
  ↓ [NEW]
Find repeat-wrapper via panel[0], render panel[1]
Store promise in renderPromises["$form.panel[1]"]
  ↓
Worker → Main Thread: 'value' changes
  ↓
fieldChanged() for radio value
  ↓ [NEW]
Detect: value change + radio + pending render
  ↓
Wait for: render + requestAnimationFrame
  ↓
repeat.js updates names: "radio" → "radio-1"
  ↓
Retry fieldChanged() → process value with correct name ✅

Expected Results:

  • ✅ 2 panel instances visible in DOM
  • ✅ All values prefilled correctly
  • ✅ Radio buttons don't interfere across instances
  • ✅ Each instance has unique names (e.g., radio, radio-1)

Fix #124

Test URLs:

Copy link

aem-code-sync bot commented Oct 7, 2025

Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed.
In case there are problems, just click a checkbox below to rerun the respective action.

  • Re-run PSI checks
  • Re-sync branch
Commits

Copy link

aem-code-sync bot commented Oct 7, 2025

Page Scores Audits Google
📱 / PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
🖥️ / PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

result.added.forEach((item) => {
this.notifyDependents(propertyChange('items', item.getState(), null));
item.dispatch(new Initialize());
});
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes should go in af2-webruntime

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Form Prefill only creates one entry of a repeatable panel...
2 participants