Skip to content

[Studio] Fix mapping rows showing a neighbouring row's values after add/remove - #679

Merged
kingjia90 merged 2 commits into
pimcore:2026.2from
alexbaat:bugfix/mapping-stale-item-values
Aug 18, 2026
Merged

[Studio] Fix mapping rows showing a neighbouring row's values after add/remove#679
kingjia90 merged 2 commits into
pimcore:2026.2from
alexbaat:bugfix/mapping-stale-item-values

Conversation

@alexbaat

@alexbaat alexbaat commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Bug

In the Studio Data Importer mapping step, the mapping rows show the wrong values as soon as the list structure changes:

  • Adding a mapping — the new mapping is inserted at the top, but the row below it also renders the new mapping's label, source column and data target.
  • Removing a mapping — every row below the removed one displays the values of the row underneath it; the last row ends up empty.

It is a display-only problem — the form store stays correct, so nothing wrong is saved and the display heals itself as soon as any value changes again — but it makes configuring a mapping very confusing.

Steps to reproduce

  1. Open a Data Objects importer, run a preview import so source columns are available, and configure at least three mappings with recognisable labels.
  2. Open the Mapping step and expand the first two mappings.
  3. Click New, pick a source column and confirm.
  4. The new mapping appears at the top and the row below it shows the same label, source and (empty) data target. In the sources panel on the left the picked column still shows a usage count of 1, confirming only one mapping actually uses it.
  5. Delete the mapping again: the first row now shows the second mapping's values, and so on down the list.

Verified on 2026.1 and reproducible on 2026.2 / 2026.x (the file is identical on all three).

Cause

MappingItemWithFilter watched its own mapping with a dynamic name path:

const itemByIndex = Form.useWatch(['mappingConfig', fieldIndex]) as MappingConfigItem | undefined

rc-field-form's useWatch registers its watcher exactly once — the effect's dependency list is [isValidForm] — and warns in development that this is unsupported: `useWatch` is not support dynamic `namePath`. Please provide static instead.

Form.List keeps field.key stable but shifts field.name (the array index) when an item is inserted or removed, and notifyWatch fires synchronously, before React re-renders. So on add(item, 0) the row that was at index 0 still has fieldIndex === 0 at notification time, reads mappingConfig[0] — the new item — and stores it. The subsequent re-render hands it fieldIndex === 1, but nothing re-reads the value, so the stale one stays on screen until the next store change. remove(index) shifts the values of all rows below it the same way.

Fix

Subscribe through a selector that reads the index from a ref — that keeps the single registration valid for value edits at a stable index — and read the value from the form during render, so it always belongs to the current index:

const fieldIndexRef = useRef(fieldIndex)
fieldIndexRef.current = fieldIndex

Form.useWatch((values: DataImporterFormValues) => values?.mappingConfig?.[fieldIndexRef.current])

const item: MappingConfigItem =
  (form.getFieldValue(['mappingConfig', fieldIndex]) as MappingConfigItem | undefined) ?? {}

Structural changes always re-render this component (field.name changes, so MappingsPanelContent rebuilds its item list), which means the fresh read cannot go stale — a missed or mis-targeted notification can now only cost one extra render, never show a wrong value.

Watching the whole mappingConfig array instead would also be correct but would re-render and re-JSON.stringify every row on every keystroke, undoing the existing per-item subscription optimisation in MappingsPanel / MappingsPanelContent.

Second commit (independent, easy to drop)

handleRemoveItem called remove(index) from inside the setActiveFilter updater. State updaters must be pure, and React invokes them twice under StrictMode (which Studio enables), so in a development build one click on the delete icon removed two mappings. The current filter is now read from the existing activeFilterRef, with the filter reset and the removal both performed outside the updater.

Notes

  • Source-only change: assets/studio has no jest/vitest setup, so there is no automated test to add here, and the built assets under src/Resources/public/studio/build/ are left to the automatic frontend build (the commit-build job is skipped for fork PRs).
  • Verified manually against a 2026.1 installation with the patched bundle rebuilt: adding a mapping no longer touches the row below it, and deleting one leaves the remaining rows intact.

Tracking issue

Fixes pimcore/platform-version#337

…dd/remove

`MappingItemWithFilter` watched its own mapping with a dynamic name path:

    Form.useWatch(['mappingConfig', fieldIndex])

rc-field-form registers a watcher exactly once (the effect's dependency list is
[isValidForm]) and warns in development that a dynamic namePath is unsupported.
`Form.List` keeps `field.key` stable but shifts `field.name` on add/remove, and
`notifyWatch` fires synchronously before React re-renders — so every row below the
change reads the store at its previous index and keeps that value: adding a mapping
made the row below it show the new mapping's label, sources and data target, and
removing one shifted the displayed values of all rows below it up by one.

Subscribe through a selector that reads the index from a ref (keeping the single
registration valid for value edits) and read the value from the form during render,
so it always belongs to the current index. Structural changes always re-render this
component, so the fresh read cannot go stale.
`handleRemoveItem` called `remove(index)` inside the `setActiveFilter` updater. State
updaters must be pure — React invokes them twice in StrictMode, so in a development
build a single click on the delete icon removed two mappings.

Read the current filter from the existing `activeFilterRef` instead and perform both
the filter reset and the removal outside the updater.
Copilot AI balanced review requested due to automatic review settings August 17, 2026 12:45
@pimcore-deployments
pimcore-deployments marked this pull request as draft August 17, 2026 12:45
@sonarqubecloud

Copy link
Copy Markdown

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes stale mapping-row values after list insertions/removals and prevents duplicate deletions under React StrictMode.

Changes:

  • Reads mapping values from the current form index while maintaining an index-aware watcher.
  • Moves form mutation outside the state updater.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
mapping-step.tsx Makes mapping removal StrictMode-safe.
mapping-item-with-filter.tsx Prevents stale values after index shifts.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot AI commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

One or more custom setup steps configured for this repository failed during this Copilot code review run:

Install dependencies with Composer

Setup steps run before each review. If the review above is missing context, or no review was posted at all, the failing step above may be the cause. See the workflow run for failure details, fix your setup steps configuration, and re-request a review.

Note

You can configure setup steps for Copilot code review separately from Copilot cloud agent with a copilot-code-review.yml file. Read the docs for details.

@alexbaat
alexbaat marked this pull request as ready for review August 17, 2026 12:49
@kingjia90 kingjia90 self-assigned this Aug 18, 2026
@kingjia90 kingjia90 added this to the 2026.2.4 milestone Aug 18, 2026
@kingjia90 kingjia90 added Bug and removed External labels Aug 18, 2026
@kingjia90
kingjia90 merged commit bd745fb into pimcore:2026.2 Aug 18, 2026
24 of 37 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Aug 18, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants