Repository navigation
feat(visual-builder): notify the editor when the entries on the page change - #662
Conversation
…648) * feat: notify the editor when the entries on the page change 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> * refactor: rename the push event to entries-in-current-page-changed Matches the noun of get-entries-in-current-page and the past-tense form the other push events use. Settled before release since the string is the wire contract on a pinned SDK. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix: notify entry changes when data-cslp is rewritten in place The page observer only watched childList, so a framework re-render that keeps the node and swaps its data-cslp (variant switch, re-keyed list) never pushed the new entry set. Watch that one attribute too. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix: keep get-entries-in-current-page deduped by entry uid The first cut keyed the page scan by entry+locale, which changed what existing callers of the request receive. An entry renders in one locale per page, so key by uid again and let the push send the same object. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix: report an empty entry set on the first page scan The initial signature was "", the same as an empty page, so a page with no data-cslp elements never sent the opening event. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * test: expect the entries-in-current-page-changed event on builder init The builder now reports an empty entry set on the first scan, so the init test sees one more post message. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> --------- 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 |
🔒 Security Scan Results
⏱️ SLA Breach Summary
✅ BUILD PASSED - All security checks passed |
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||
hitesh-shetty-cstk
left a comment
There was a problem hiding this comment.
Automated review
What this changes: The SDK now pushes the set of entries rendered on the page to the editor instead of waiting to be asked. A new entries-in-current-page-changed event is sent once the builder is ready and again from the existing body MutationObserver whenever the set changes, gated by an order-independent signature so an unchanged set sends nothing. The observer also starts watching data-cslp attribute changes. The existing request-response event is left in place.
Business impact: Visual Builder canvas boot. The new notification is inserted into the builder init sequence, and the body observer that drives the focus overlay and empty-block generation now has a wider trigger. If the inserted call throws, the init steps after it do not run, and an editor that stops receiving the push falls back to the stale list it last pulled. A content editor would see inline editing affordances stop refreshing, or changes made elsewhere not reflected on the canvas. ZuhairAhmed-cs has touched src/visualBuilder/index.ts most (7 of the last 30 commits on the base branch). Please walk them through the init-path and observer changes before merging.
Security: Nothing beyond what the scanners cover. The push sends entry uid, content type uid, and locale to the parent frame, which is the same data the existing request-response event already returns to the same parent over the same channel, so this changes when the data is sent rather than who can obtain it. Snyk and the org security workflow are both clean on this head, and no workflow files are in the diff.
Flow
sequenceDiagram
participant DOM as Host page DOM
participant OBS as body MutationObserver
participant SDK as VisualBuilder (iframe)
participant ED as Editor (parent)
SDK->>SDK: builder ready
SDK->>ED: entries-in-current-page-changed
Note over SDK: changed here
DOM->>OBS: childList or data-cslp change
Note over OBS: changed here, data-cslp is new
OBS->>SDK: debounced 100ms callback
SDK->>SDK: signature equal? return
SDK->>ED: entries-in-current-page-changed
ED->>SDK: get-entries-in-current-page (unchanged)
Findings: 0 blocker, 3 should fix, 1 nit. All four are inline.
Reviewer candidates: ZuhairAhmed-cs authored 7 of the last 30 commits on src/visualBuilder/index.ts, the file carrying the init and observer changes. Deepak-Kharah is one of three authors on src/visualBuilder/utils/getEntryIdentifiersInCurrentPage.ts, the file this PR rewrites. The account running this review is the top author across the changed files but is excluded from its own candidate list.
Not covered: No tests, build, or E2E run were executed. The send rejection contract was read from this repo's own utils/postMessageErrors.ts rather than the post-message library source, which is not installed in this checkout. The editor-side branch this was tested against is not visible from here, so the claim that the editor handles the new event is unverified.
Automated review by Claude Code. A human review is still required.
Generated by Claude Code
The push had no rejection handler, so an editor without the listener left an unhandled rejection on every send and the set was never resent. The failed signature now resets so the next observer pass retries, and destroy() cancels the pending debounced pass. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
🔒 Security Scan Results
⏱️ SLA Breach Summary
✅ BUILD PASSED - All security checks passed |
hitesh-shetty-cstk
left a comment
There was a problem hiding this comment.
Automated review
Re-review after new commits (pass 2 of 4).
Canvas editing is the journey at risk. This commit rewires the callback behind the body MutationObserver, which also drives the focus overlay refresh and empty modular-block rendering, not only the new entries event. Wired to an undefined field it would throw during construction and a user would get an iframe that renders the site with no edit overlays at all. The declaration order is correct as written, so this is about how easily the wiring breaks rather than a defect today.
What this changes: Three additions on top of the base change. The entries send gains a rejection handler that clears the cached signature so a later observer pass can resend. The debounced observer callback moves out of the MutationObserver constructor into a named class field so destroy() can cancel a pass that is already queued. A new unit test file covers the resend after a rejected send, the absence of a send on an unrelated mutation, and the cancel on destroy.
The rejection handling and the cancel() on teardown both answer points raised on the previous pass.
Business impact: Canvas inline editing and canvas boot, as above. The failure mode this commit targets is narrower: when the editor is not yet listening at init, the entry list is dropped and, on a page that does not mutate again, never resent. The editor can still pull the list, so an editor sees stale entry data rather than a broken canvas.
Security: nothing beyond what the scanners cover. The change only sends outbound; no new payload is read from the parent.
Flow
sequenceDiagram
participant DOM as Page body
participant VB as VisualBuilder (SDK, in iframe)
participant ED as Editor (parent window)
VB->>VB: init, notifyEntriesInPageIfChanged()
VB->>ED: ENTRIES_IN_CURRENT_PAGE_CHANGED
ED--)VB: reject, no listener yet
Note over VB: changed here: clear cached signature
DOM->>VB: later mutation, debounced 100ms
VB->>VB: notifyEntriesInPageIfChanged()
VB->>ED: ENTRIES_IN_CURRENT_PAGE_CHANGED (resent)
Note over VB: changed here: destroy() cancels a queued pass
The gap flagged inline: when no later mutation arrives, the resend step never runs.
Findings: 0 blocker, 4 should fix, 1 nit. All five are inline.
Reviewer candidates: ZuhairAhmed-cs authored 7 of the last 30 commits on src/visualBuilder/index.ts and is the person to walk through the observer rewiring. No reviewer is requested on a re-review, so this is a name to consult, not an assignment. The test file is new and has no history to draw a second name from.
Not covered: No tests, build, or E2E run were executed. The editor side is not visible from this repository, so whether a resent event is handled the same way as the first is unverified. The claim that recovery works was read from the code rather than observed; the static-page gap is the part of that reasoning worth checking against a real page.
Automated review by Claude Code. A human review is still required.
Generated by Claude Code
Summary
Brings the realtime epic branch into
develop_v4. It carries one change, already reviewed and approved in #648: the SDK 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, so late-rendered references (lazy sections, client-side fetches) never reached it.Changes
ENTRIES_IN_CURRENT_PAGE_CHANGEDpost-message event, sent once the builder is ready and again from the existing bodyMutationObserverwhenever the entry set on the page changes. The observer also watchesdata-cslpattribute changes, since frameworks rewrite it in place on variant switches and re-keyed lists.getEntryIdentifiersInCurrentPagededups by entry uid (an entry renders in one locale per page). The signature that gates the push is keyed by uid and locale, so a locale switch on the same entries is still reported.develop_v4without conflicts.Test plan
npm test -- getEntryIdentifiersInCurrentPagecovers dedup and the signature.npm test -- index.testcovers the opening event on a page with no entries.🤖 Generated with Claude Code