Conversation
Send entries-in-page-changed once the builder is ready and again from the body MutationObserver whenever the entry+locale set on the page changes, so the editor can react to late-rendered references without polling. Dedup the page scan by entry and locale. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
🔒 Security Scan Results
⏱️ SLA Breach Summary
✅ BUILD PASSED - All security checks passed |
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||
sairaj-cs
changed the title
kirtesh-cstk
left a comment
kirtesh-cstk
left a comment
There was a problem hiding this comment.
Reviewed alongside visual-builder#2717. The direction is right and the push carries its payload, which is already better than the send-variant-and-locale pattern it sits next to (that one pings, then the editor sends two more messages back). Comments below are one naming call and two edge cases. Nothing blocking the approach.
|
|
||
| // FROM visual builder | ||
| GET_ALL_ENTRIES_IN_CURRENT_PAGE = "get-entries-in-current-page", | ||
| ENTRIES_IN_PAGE_CHANGED = "entries-in-page-changed", |
There was a problem hiding this comment.
Can we make this entries-in-current-page-changed?
get- is the request/response prefix everywhere in this enum, and each of those is a .send() that awaits a value. Push notifications use the unprefixed past-tense form (composition-saved, primary-composition-detected). Matching get-entries-in-current-page on the noun also makes the pair obvious when reading the enum.
Worth settling now: this string is the wire contract on an SDK that customers pin, so renaming after release means supporting both forever.
| this.visualBuilderContainer, | ||
| this.resizeObserver | ||
| ); | ||
| this.notifyEntriesInPageIfChanged(); |
There was a problem hiding this comment.
The observer is { childList: true, subtree: true }, so attribute-only changes never reach notifyEntriesInPageIfChanged.
useRecalculateVariantDataCSLPValues rewrites data-cslp in place on a variant switch. Same nodes, different entry uids, no mutation record, no push. The editor would keep the old entries' channels and miss the new ones.
Either add attributes: true, attributeFilter: ["data-cslp"] to the observe call, or call notifyEntriesInPageIfChanged() at the end of the recalculation path.
| locale: cslpData.locale | ||
| } | ||
| ); | ||
| uniqueEntriesMap.set(`${cslpData.entry_uid}.${cslpData.locale}`, { |
There was a problem hiding this comment.
The dedup key moved from entry_uid to entry_uid.locale, which changes what the existing get-entries-in-current-page response returns, not just the new event. On a page rendering one entry in two locales, every current caller now gets that entry twice.
Correct for channel subscriptions, but the callers were written against the old shape. Worth a check on EntryAccordionList and ContentPublishModal before this ships, since a duplicate in the publish list is the expensive kind of surprise. If they are not locale-aware, the signature could key on entry+locale while the response stays deduped by uid.
2 participants
Summary
The SDK now tells the editor which entries are on the page and pushes an update whenever that set changes. Until now the editor had to ask for the list and hope it was still current. Late-rendered references (lazy sections, client-side fetches) never reached it.
Changes
ENTRIES_IN_PAGE_CHANGEDpost-message event, sent once the builder is ready and again from the existing bodyMutationObserverwhenever the set of entry+locale pairs on the page changes.getEntryIdentifiersInCurrentPagededups by entry and locale and exposes a signature so the observer only posts on a real change.Test plan
npm test -- getEntryIdentifiersInCurrentPagecovers dedup and the signature.🤖 Generated with Claude Code