Skip to content

fix(sdk): stop leaking UVE edit-mode data-dot-* attributes into live pages (#36095)#36288

Merged
zJaaal merged 3 commits into
mainfrom
issue-36095-sdk-strip-data-dot-live-mode
Jun 24, 2026
Merged

fix(sdk): stop leaking UVE edit-mode data-dot-* attributes into live pages (#36095)#36288
zJaaal merged 3 commits into
mainfrom
issue-36095-sdk-strip-data-dot-live-mode

Conversation

@zJaaal

@zJaaal zJaaal commented Jun 23, 2026

Copy link
Copy Markdown
Member

Proposed Changes

Fixes #36095 — the dotCMS SDKs (React, Angular, JS) shared rendering logic that always emitted the full set of UVE editor metadata onto container and contentlet wrappers, so data-dot-* attributes leaked into live / production output. This bloats public markup and exposes internal identifiers.

Behavior by mode

Edit (UVE) Live + Analytics active Live, no Analytics
Contentlet full editor set + data-dot-object only data-dot-identifier/inode/title/type/basetype + dotcms-contentlet class class only — zero data-dot-*
Container full editor set clean clean

The Analytics carve-out follows Arcadio's heads-up: impression/click tracking needs the content's identifying attributes (and the dotcms-contentlet class) in live mode. Gating is driven by window.__dotAnalyticsActive__, and the SDKs observe the dotcms:analytics:ready event so attributes appear regardless of analytics initialization order. No new customer configuration required.

Implementation

  • @dotcms/uve (shared): getAnalyticsContentletAttributes() (minimal set), isDotAnalyticsActive(), and ANALYTICS_ACTIVE_WINDOW_KEY / ANALYTICS_READY_EVENT constants — kept in sync with @dotcms/analytics.
  • @dotcms/react: Contentlet/Container gate attributes by mode; new useIsAnalyticsActive hook reacts to the ready event.
  • @dotcms/angular: contentlet/container use reactive host bindings; DotCMSStore exposes $isAnalyticsActive derived from the ready event via fromEvent + toSignal (no NgZone juggling; safe on the SDK's min Angular >=17).
  • @dotcms/types: DotAnalyticsContentletAttributes.

Angular

Screen.Recording.2026-06-23.at.1.copy.mp4

NextJS

Screen.Recording.2026-06-23.at.1.mp4

Testing

  • Regression tests across sdk-uve, sdk-react, sdk-angular covering all three modes (edit / live-no-analytics / live-with-analytics) — nx run-many -t test lint green for all four projects.
  • Manually verified the rendered live DOM in the Next.js and Angular example apps (against a local build): no data-dot-* in live mode; the minimal content attributes reappear when analytics activates; full set returns in UVE edit mode.

🤖 Generated with Claude Code

This PR fixes: #36095

…pages (#36095)

The React, Angular and JS SDKs share rendering logic that always emitted
the full set of UVE editor metadata (data-dot-object, data-dot-identifier,
data-dot-container, data-dot-uuid, etc.) on container and contentlet
wrappers, even when a page is served in live mode. This bloats the public
markup and leaks internal identifiers.

Live mode now strips this metadata:
- Container wrappers emit no data-dot-* attributes outside edit mode.
- Contentlet wrappers emit nothing by default; only when DotCMS Analytics
  is active do they keep the minimal set it needs to identify content
  (data-dot-identifier/inode/title/type/basetype) plus the dotcms-contentlet
  class, per Arcadio's heads-up. The dotcms:analytics:ready event is observed
  so attributes appear regardless of analytics init order.
- Edit mode (UVE) is unchanged and still emits the full attribute set.

Shared helpers (getAnalyticsContentletAttributes, isDotAnalyticsActive) and
constants live in @dotcms/uve so all SDKs stay in sync. Adds regression tests
across uve, react and angular.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added Area : Frontend PR changes Angular/TypeScript frontend code Area : SDK PR changes SDK libraries labels Jun 23, 2026
@claude

claude Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Claude finished @zJaaal's task in 1m 41s —— View job


Rollback Safety Analysis

  • Read rollback-unsafe categories reference
  • Get full PR diff
  • Analyze diff against unsafe categories
  • Apply appropriate label

Result: ✅ Safe To Rollback

All 17 changed files are confined to frontend SDK packages under core-web/libs/sdk/:

Package Files changed
@dotcms/angular container + contentlet components, store
@dotcms/react Container, Contentlet, new useIsAnalyticsActive hook
@dotcms/uve dom utilities + constants
@dotcms/types type declarations

No unsafe categories match:

Category Verdict
C-1 Structural DB model change ❌ No DB changes
C-2 Elasticsearch mapping change ❌ No ES changes
C-3 Content JSON model version bump ❌ No Java serialization changes
C-4 DROP TABLE / DROP COLUMN ❌ No SQL
H-1 One-way data migration ❌ No data migrations
H-2 RENAME TABLE / COLUMN ❌ No SQL
H-3 PK restructuring ❌ No SQL
H-4 New field type ❌ No new Java field types
H-5 Binary storage provider change ❌ No storage changes
H-6 DROP PROCEDURE / FUNCTION ❌ No SQL
H-7 NOT NULL column ❌ No schema changes
H-8 VTL viewtool contract change ❌ No VTL/Java viewtool changes
M-1 Column type change ❌ No SQL
M-2 Push publishing bundle format ❌ No bundler changes
M-3 REST/GraphQL API contract change ❌ No backend API changes
M-4 OSGi interface breakage ❌ No OSGi changes

The changes are purely frontend SDK output changes — they alter which HTML attributes are rendered client-side based on rendering mode (edit vs. live). Rolling back to the previous release restores the prior attribute-emission behavior with no data loss, no reindexing, and no backend impact.

Label added: AI: Safe To Rollback

@github-actions

github-actions Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

🤖 Bedrock Review — deepseek.v3.2

[🟠 High] core-web/libs/sdk/angular/src/lib/components/dotcms-layout-body/components/contentlet/contentlet.component.ts:96 — The getContainerAttribute() method is called in the host binding ('[attr.data-dot-container]': '$isDevMode() ? getContainerAttribute() : null'), but getContainerAttribute is not a signal or observable. This could cause change detection issues because Angular may not re-evaluate the binding when containerData changes. Use a computed signal instead.

[🟡 Medium] core-web/libs/sdk/angular/src/lib/components/dotcms-layout-body/components/contentlet/contentlet.component.ts:79 — The $dotAttributes computed signal returns {} as DotContentletAttributes when !contentlet. However, DotContentletAttributes expects specific required keys (e.g., 'data-dot-identifier'). Returning an empty object cast to this type could lead to runtime errors if something expects those keys. Return a properly typed empty object or adjust the type.

[🟡 Medium] core-web/libs/sdk/angular/src/lib/store/dotcms.store.ts:50 — The $isAnalyticsActive signal uses toSignal with fromEvent. If window is undefined (server-side), it defaults to signal(false). However, the fromEvent import and toSignal are still evaluated on the server, which could cause issues. Ensure the fromEvent import is side-effect safe or use a conditional import pattern.

[🟡 Medium] core-web/libs/sdk/react/src/lib/next/components/Contentlet/Contentlet.tsx:70 — In the dotAttributes memo, when isDevMode is true, the spread includes getDotContentletAttributes(...) and adds 'data-dot-object': 'contentlet'. However, getDotContentletAttributes already returns an object with 'data-dot-object': 'contentlet' (as seen in the Angular implementation). This could cause a duplicate key or override. Verify that getDotContentletAttributes does not already include data-dot-object; if it does, remove the explicit addition to avoid conflicts.

[🟡 Medium] core-web/libs/sdk/react/src/lib/next/components/Container/ContainerFallbacks.tsx:48EmptyContainer now accepts Partial<DotContainerAttributes> instead of DotContainerAttributes. This is a breaking change for any external caller passing the full type. Ensure no existing callers are affected (likely internal only, but review).

[🟡 Medium] core-web/libs/sdk/uve/src/lib/dom/dom.utils.ts:325isDotAnalyticsActive() checks for a window flag ANALYTICS_ACTIVE_WINDOW_KEY. The comment says it's duplicated from @dotcms/analytics. If the constant value changes in the analytics package, this will break. Consider importing the constant from a shared package or adding a synchronization test.

[🟡 Medium] core-web/libs/sdk/uve/src/lib/dom/dom.spec.ts:394 — The test for getAnalyticsContentletAttributes expects 'data-dot-basetype': 'test-base', but the implementation uses contentlet?.baseType (capital 'T'). The mock contentlet uses baseType: 'test-base' (lowercase 't'). This could cause a test failure if the property name doesn't match. Verify the property name in DotCMSBasicContentlet is baseType (capital 'T').

[🟡 Medium] core-web/libs/sdk/angular/src/lib/components/dotcms-layout-body/components/container/container.component.spec.ts:48 — The test uses as unknown as to cast the injected store. This bypasses type safety. Consider using a proper mock type or adjusting the provider to avoid casting.

[🟡 Medium] core-web/libs/sdk/angular/src/lib/components/dotcms-layout-body/components/contentlet/contentlet.component.spec.ts:49 — The test helper hostAttr uses as string | null casting. This is fine, but ensure it doesn't hide actual null returns from getAttribute (which can return null). It's acceptable.

[🟡 Medium] core-web/libs/sdk/react/src/lib/next/__test__/hook/useIsAnalyticsActive.test.tsx:47 — The test spies on window.removeEventListener but doesn't mock it globally. If other tests modify removeEventListener, this could cause interference. Use a jest spy on the specific instance or clean up properly.


Run: #28049574607 · tokens: in: 14198 · out: 1013 · total: 15211

@zJaaal zJaaal requested review from KevinDavilaDotCMS and oidacra and removed request for KevinDavilaDotCMS June 23, 2026 18:55
@github-actions

github-actions Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

🤖 Bedrock Review — qwen.qwen3-next-80b-a3b

#### New Issues
- 🔴 Critical: `core-web/libs/sdk/angular/src/lib/components/dotcms-layout-body/components/contentlet/contentlet.component.ts:76-85` — `$dotAttributes()` returns analytics attributes even when `$isDevMode()` is true, overriding editor metadata. This corrupts edit mode UI by replacing full editor attributes with minimal analytics ones, breaking content editing.
- 🔴 Critical: `core-web/libs/sdk/react/src/lib/next/components/Contentlet/Contentlet.tsx:70-75` — In edit mode, `getAnalyticsContentletAttributes` is never called, but `getDotContentletAttributes` is augmented with `'data-dot-object': 'contentlet'`. This omits critical editor attributes like `data-dot-container` and `data-dot-on-number-of-pages`, breaking UVE functionality.
- 🟠 High: `core-web/libs/sdk/angular/src/lib/store/dotcms.store.ts:33-39` — `$isAnalyticsActive` signal is initialized with `isDotAnalyticsActive()` but never updates if `window` is undefined. This causes a memory leak in SSR environments where `window` is undefined and the signal is permanently stuck at `false`.
- 🟠 High: `core-web/libs/sdk/uve/src/lib/dom/dom.utils.ts:294-304` — `isDotAnalyticsActive()` reads `__dotAnalyticsActive__` as a strict `=== true` check but does not validate type. If `__dotAnalyticsActive__` is set to `"true"` (string), it returns `false`, silently breaking analytics attribution in live mode.
- 🟡 Medium: `core-web/libs/sdk/react/src/lib/next/hooks/useIsAnalyticsActive.ts:28-29` — `useIsAnalyticsActive` removes event listener on unmount but does not guard against duplicate removal. If hook unmounts twice (e.g., during route transition), it throws an error in strict mode.

#### Existing
- 🟡 Medium: `core-web/libs/sdk/uve/src/internal/constants.ts:147-148` — `ANALYTICS_ACTIVE_WINDOW_KEY` and `ANALYTICS_READY_EVENT` are duplicated from `@dotcms/analytics` with no versioned dependency or build-time validation. Risk of drift if analytics package updates independently.

#### Resolved
- ✅ `core-web/libs/sdk/angular/src/lib/components/dotcms-layout-body/components/container/container.component.spec.ts:35-47` — Test now correctly asserts attribute behavior in dev/live mode — no longer relies on deprecated `jest.fn()` mocks.
- ✅ `core-web/libs/sdk/angular/src/lib/components/contentlet/contentlet.component.spec.ts:46-83` — Tests refactored to use `hostAttr` helper and separate modes — no longer conflates dev/live behavior.

<!-- dotcms-review-findings:[{"sev":"🔴 Critical","loc":"core-web/libs/sdk/angular/src/lib/components/dotcms-layout-body/components/contentlet/contentlet.component.ts:76-85","desc":"$dotAttributes() returns analytics attributes even when $isDevMode() is true, overriding editor metadata and breaking UVE editing."},{"sev":"🔴 Critical","loc":"core-web/libs/sdk/react/src/lib/next/components/Contentlet/Contentlet.tsx:70-75","desc":"In edit mode, getAnalyticsContentletAttributes is never called and getDotContentletAttributes is augmented with only data-dot-object, omitting critical editor attributes like data-dot-container and data-dot-on-number-of-pages."},{"sev":"🟠 High","loc":"core-web/libs/sdk/angular/src/lib/store/dotcms.store.ts:33-39","desc":"$isAnalyticsActive signal is initialized with isDotAnalyticsActive() but never updates if window is undefined, causing permanent false state in SSR environments."},{"sev":"🟠 High","loc":"core-web/libs/sdk/uve/src/lib/dom/dom.utils.ts:294-304","desc":"isDotAnalyticsActive() reads __dotAnalyticsActive__ as strict === true without type validation, silently breaking analytics attribution if value is string \"true\"."},{"sev":"🟡 Medium","loc":"core-web/libs/sdk/react/src/lib/next/hooks/useIsAnalyticsActive.ts:28-29","desc":"useIsAnalyticsActive removes event listener on unmount but does not guard against duplicate removal, risking error in strict mode during rapid route transitions."},{"sev":"🟡 Medium","loc":"core-web/libs/sdk/uve/src/internal/constants.ts:147-148","desc":"ANALYTICS_ACTIVE_WINDOW_KEY and ANALYTICS_READY_EVENT are duplicated from @dotcms/analytics with no versioned dependency or build-time validation, risking drift if analytics package updates independently."}] -->

Run: #28103167087 · tokens: in: 14011 · out: 1004 · total: 15015

@zJaaal

zJaaal commented Jun 24, 2026

Copy link
Copy Markdown
Member Author

Re: the two automated Bedrock reviews (deepseek + qwen) — I went through every finding against the committed code; none are actionable. Summary:

  • "Edit mode returns analytics attrs / omits editor attrs" (Critical ×2) — incorrect. Both SDKs check dev/edit mode first and emit the full editor set there (React spreads getDotContentletAttributes(...) then adds data-dot-object; Angular's $dotAttributes returns the full set when $isDevMode()). The analytics-minimal path is only reached in live mode. Asserted by the new edit-mode tests.
  • SSR memory leak in the store — the typeof window === 'undefined' branch returns signal(false) with no subscription; the client branch uses toSignal, which auto-unsubscribes via DestroyRef.
  • === true vs string — the flag is a boolean internal contract set by @dotcms/analytics.
  • removeEventListener throwing on double-unmount — it's idempotent.
  • getContainerAttribute() host binding not reactive — host-binding expressions re-evaluate every CD; an @Input change triggers CD. A computed over a non-signal input would actually be less reactive here.
  • constant duplication — same documented pattern as CONTENTLET_CLASS.

— posted by Claude on behalf of @zJaaal

@mergify

mergify Bot commented Jun 24, 2026

Copy link
Copy Markdown

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

@zJaaal zJaaal added this pull request to the merge queue Jun 24, 2026
Merged via the queue into main with commit 7252072 Jun 24, 2026
40 checks passed
@zJaaal zJaaal deleted the issue-36095-sdk-strip-data-dot-live-mode branch June 24, 2026 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI: Safe To Rollback Area : Frontend PR changes Angular/TypeScript frontend code Area : SDK PR changes SDK libraries

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

SDK: UVE edit-mode data-dot-* attributes leak into live pages

2 participants