Calling the `setPageData` action creator directly doesn't update values in the page data form

Created on 2 July 2025, 25 days ago

Overview

Calling the setPageData action creator of the pageDataSlice directly doesn't update values in the page data form. This was discovered in 📌 Generate content for title using AI Active .

Proposed resolution

My theory is that inputBehaviors initially sets the default value when the form element is rendered, but it's not reactive to changes that happen in pageDataSlice. This has gone unnoticed because previously changes were always carried out by users using the form, so the form would already reflect the most up-to-date values.

User interface changes

n/a

🐛 Bug report
Status

Active

Version

0.0

Component

Redux-integrated field widgets

Created by

🇳🇱Netherlands balintbrews Amsterdam, NL

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @balintbrews
  • 🇳🇱Netherlands balintbrews Amsterdam, NL

    The current changes in the MR fix the problem, but only without React.StrictMode (can be removed in src/main.tsx). React.StrictMode renders every component twice, and that must be causing a race condition. This will only affect the app in development mode, but it's still a bug we need to fix.

  • Pipeline finished with Failed
    25 days ago
    Total: 2344s
    #537276
  • 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10

    My theory is that inputBehaviors initially sets the default value when the form element is rendered, but it's not reactive to changes that happen in pageDataSlice. This has gone unnoticed because previously changes were always carried out by users using the form, so the form would already reflect the most up-to-date values.

    Yes, the value shown on the field is set internally in a useState, the values in the pageDataSlice don't go back the other way

  • 🇳🇱Netherlands balintbrews Amsterdam, NL

    #4: We're using a selector from pageDataSlice, so we're already subscribed to changes, but the internal state was not being updated. That's already fixed, now the suspected race condition remains (see #3).

  • 🇺🇸United States bnjmnm Ann Arbor, MI

    Talked to @balintbrews about me having a crack at this since I've been working in this corner of XB quite often lately.

    I'm leaning towards creating an additional action that is very similar to setPageData but specifically for use cases where the store change needs to inform the form vs the other (more common) way around.

  • 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10

    We could use a custom event, we do something similar for when ajax updates the form values, albeit to update the page data slice, but input behaviours could generically listen for an event that tells it to update its internal value

  • 🇳🇱Netherlands balintbrews Amsterdam, NL

    I wonder why it is better to introduce a new API when we already have the pageDataSlice, to which inputBehaviors is already subscribed via a selector. My current MR simply adds logic to react to those changes and update the internal state.

    I'm not attached to that approach, but aren't we painting over a bug <React.StrictMode> is showing us with the inconsistent re-renders?

  • 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10

    Ah sorry, I missed that we took value from pageData before passing it to inputBehaviorsCommon - that seems like a clean approach

  • Pipeline finished with Running
    18 days ago
    #542351
  • Merge request !1250#3533703 Refresh entire form on programattic update → (Merged) created by bnjmnm
  • 🇺🇸United States bnjmnm Ann Arbor, MI

    I could not get the original MR: 3533703-setpagedata-vs-form-values to work. I'm not sure what use cases it was able to work as the defaultValue (AFAIK) does not consult the pageData slice.

    I tested that MR and my new one by installing the xb_test_article_fields module and adding a temporary button that calls setPageData

    <button
            style={{ marginTop: '20px', border: '3px solid black' }}
            onClick={() => {
              dispatch(
                setPageData({
                  ...formState,
                  'body[0][value]': 'stringForTesting body field',
                  'body[0][summary]': 'stringForTesting summary',
                  'field_xbt_textarea[0][value]':
                    '<p><b>stringForTesting bold</b></p>',
                  'field_xbt_textarea_summary[0][value]':
                    'stringForTesting textarea summary value',
                  'field_xbt_textarea_summary[0][summary]':
                    'stringForTesting textarea summary summary',
                  field_xbt_options_buttons: 'option3',
                  'path[0][langcode]': 'zxx',
                  'field_xbt_textfield[0][value]':
                    'stringForTesting textfield value',
                  'field_xbt_unlimited_text[0][value]':
                    'stringForTesting on the Coast',
                }),
              );
            }}
          >
            TEMPORARY BUTTON THAT UPDATES PAGE DATA
          </button>

    In the 3533703-setpagedata-vs-form-values-refetch-on-update MR, this temporary button is also there but calls a newly added updatePageData action which is just setPageData but also triggers an event to inform the page data form it needs to refresh. I've confirmed this approach works with text fields, but not sure about radio/select etc yet.

    It seems like there was some doubt about my approach so perhaps the ultimate solution is some combination of the two MRs?

  • Pipeline finished with Failed
    18 days ago
    #542412
  • 🇳🇱Netherlands balintbrews Amsterdam, NL

    the defaultValue (AFAIK) does not consult the pageData slice

    My understanding is that it does, but I could easily be wrong. 🙂 Here is how I traced it:

    1. In InputBehaviorsEntityForm, we subscribe to selectPageData;
    2. pageData[fieldName] indirectly ends up as attributes.value or attributes.checked;
    3. attributes is part of the props object that gets passed to InputBehaviorsCommon;
    4. In InputBehaviorsCommon, getDefaultValue() reads from attributes that is previously destructured from props.

    It seems like there was some doubt about my approach so perhaps the ultimate solution is some combination of the two MRs?

    Not really a doubt, I was just wondering if the inconsistent rendering by React.StrictMode shows a potential bug. You and @larowlan know this area way more than I do, so I'm happy to accept your direction.

  • Merge request !1253Draft: Issue #3533703: Subscribe to updates → (Open) created by larowlan
  • 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10

    Found the issue we were overwriting the attribute.value in InputBehaviorsCommon, new MR up with a fix

  • Pipeline finished with Failed
    18 days ago
    Total: 752s
    #542717
  • 🇺🇸United States bnjmnm Ann Arbor, MI

    The new MR 1253 looks promising - if we can avoid more build_id headaches I'm all for it.

    I was unable to test manually until I made a change in the getPathAlias function DrupalPathWidget.tsx that checked for the truthiness of titleValue before it calls string-specific methods.

    The need to do that might be related to what I then ran into:

    While testing manually, I ran into an issue that seems to only occur if no fields in the page data form have been interacted with yet.
    To reproduce:

    • Reload the UI and immediately click the "temporary button that updates page data"
    • Note that this updates the form field, but not the preview.
    • Once any further interaction with the form takes place - regardless of field - the title field will reverts to the stale value shown in the preview.

    .

  • 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10

    Once any further interaction with the form takes place - regardless of field - the title field will reverts to the stale value shown in the preview.

    That's because the button doesn't also write to the form state slice

    Anything that uses this pattern will need to do both

  • 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10

    That's because the button doesn't also write to the form state slice

    Actually I think we could make inputBehaviors do this too

  • 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10

    Once any further interaction with the form takes place - regardless of field - the title field will reverts to the stale value shown in the preview.

        // eslint-disable-next-line react-hooks/exhaustive-deps
    

    was biting us here 😂

    Pushed a simple fix in inputBehaviours

  • Pipeline finished with Failed
    17 days ago
    Total: 1676s
    #543504
  • 🇳🇱Netherlands balintbrews Amsterdam, NL
  • Pipeline finished with Failed
    16 days ago
    Total: 203s
    #544616
  • Pipeline finished with Failed
    16 days ago
    Total: 618s
    #544618
  • Pipeline finished with Running
    16 days ago
    #545169
  • Pipeline finished with Failed
    15 days ago
    Total: 1304s
    #545411
  • Pipeline finished with Failed
    15 days ago
    Total: 878s
    #545418
  • Pipeline finished with Failed
    15 days ago
    Total: 261s
    #545437
  • Pipeline finished with Failed
    15 days ago
    Total: 3692s
    #545443
  • Pipeline finished with Success
    15 days ago
    Total: 1412s
    #545493
  • 🇺🇸United States bnjmnm Ann Arbor, MI

    MR 1250 ready

    One way to manually test is to add:

    // PageDataForm.tsx
    // update imports
    import {
      selectPageData,
      setPageData,
      updatePageDataExternally,
    } from '@/features/pageData/pageDataSlice';
    
    // add a test button
    /*
          <button
            style={{ marginTop: '20px', border: '3px solid black' }}
            onClick={() => {
              dispatch(
                updatePageDataExternally({
                  'title[0][value]': 'stringForTesting title',
                  'body[0][summary]': 'stringForTesting summary',
                }),
              );
            }}
          >
            TEMPORARY BUTTON THAT UPDATES PAGE DATA
          </button>
     */
    
    

    I'm assuming tests for the modules that wind up using this will

  • 🇫🇮Finland lauriii Finland

    I'm assuming tests for the modules that wind up using this will

    The code in the AI module is already merged. If we think this needs test coverage and if we're not adding tests here, we make sure to open a follow-up to ensure those tests get written. Unless there's some integration code needed for this.

  • 🇳🇱Netherlands balintbrews Amsterdam, NL
  • Pipeline finished with Failed
    15 days ago
    Total: 942s
    #545577
  • Automatically closed - issue fixed for 2 weeks with no activity.

Production build 0.71.5 2024