Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: collection config deep merge during sanitization causing unpredictable behavior #11524

Merged
merged 7 commits into from
Mar 4, 2025

Conversation

AlessioGr
Copy link
Member

@AlessioGr AlessioGr commented Mar 4, 2025

Deep‐merging the collection config defaults during sanitization causes all collection fields to end up with different object references. This is not only slow, but can also lead to unpredictable behavior: mutations made before collection sanitization are reflected in the field config, while mutations made afterward, using the same object reference, are not reflected in the collection’s field config.

Specifically, the following happened:

  1. A Block was defined in the module scope.
  2. It was then added to both a collection’s blocks field and the config.blocks property.
  3. Rich text sanitization promises for config.blocks were collected.
  4. The collection config was sanitized.
  5. The config.blocks sanitization promises were awaited.
  6. Rich text fields were sanitized in config.blocks, but ended up not being sanitized in the collection config referencing the same block, because the object reference held by the promise callback no longer matched the collection config’s object reference. The collection config block did not create its own rich text sanitization promise, as _sanitized: true was set on the block during the earlier config.blocks sanitization, which skipped it.

Our config defaults pattern was brittle in general. It’s easy to misuse object spreading or to mutate the config defaults later when you intended only to mutate the payload or collection config. Our current approach was vulnerable to this because it retained some object references from the config defaults.

This PR introduces reliable merge functions that are faster and ensure no object references are shared with defaults that reside in the module scope.

…ctable behavior
@AlessioGr AlessioGr enabled auto-merge (squash) March 4, 2025 20:44
@AlessioGr AlessioGr merged commit 5adb764 into main Mar 4, 2025
69 checks passed
@AlessioGr AlessioGr deleted the fix/collection-config-deepcopy branch March 4, 2025 21:02
r1tsuu added a commit that referenced this pull request Mar 5, 2025
…itization (#11534)

The PR #11512 was merged
without changes from this PR
#11524

Which caused all the tests to fail:
<img width="432" alt="image"
src="https://github.com/user-attachments/assets/7c19187e-a9c4-4dad-80c2-bdd6156eeb0b"
/>

Corrects default value sanitization with the new strategy from #11524
Copy link
Contributor

github-actions bot commented Mar 5, 2025

🚀 This is included in version v3.27.0

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

Successfully merging this pull request may close these issues.

None yet

1 participant