refactor(sdk-analytics): Consolidate analytics data attributes and simplify Contentlet component #33948
Conversation
…attributes ### Summary This commit refactors the analytics SDK by consolidating the import of analytics-related constants and updating data attribute names for consistency across the codebase. ### Changes Made - Moved `ANALYTICS_WINDOWS_ACTIVE_KEY` and `ANALYTICS_WINDOWS_CLEANUP_KEY` imports to a shared constants file for better organization. - Updated data attribute names in the impression tracker and identity tracker to use a consistent naming convention (`dotIdentifier`, `dotInode`, etc.). - Adjusted the contentlet component to apply the new class name for contentlet elements. ### Benefits - Improved code maintainability and readability by centralizing constant definitions. - Enhanced consistency in data attributes used across different components and plugins.
… impression tracker tests
There was a problem hiding this comment.
Pull request overview
This PR consolidates analytics-related constants and data attributes to simplify the architecture across SDK packages. The main goal is to eliminate duplicate analytics logic in the React Contentlet component by standardizing on UVE's data attributes (data-dot-*) instead of maintaining separate analytics-specific attributes (data-dot-analytics-*).
Key changes:
- Moved analytics window constants from
@dotcms/uve/internalto@dotcms/analyticsfor better separation of concerns - Standardized data attribute names to use UVE's
data-dot-*pattern across all analytics plugins - Removed conditional analytics state management from React's Contentlet component (now always renders UVE attributes)
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
core-web/libs/sdk/uve/src/lib/dom/dom.utils.ts | Removed getDotAnalyticsAttributes() helper function (analytics now uses standard UVE attributes) |
core-web/libs/sdk/uve/src/lib/core/core.utils.ts | Removed isAnalyticsActive() utility function (no longer needed for conditional rendering) |
core-web/libs/sdk/uve/src/lib/core/core.spec.ts | Removed 50+ lines of tests for deleted isAnalyticsActive() function |
core-web/libs/sdk/uve/src/internal/constants.ts | Removed analytics constants ANALYTICS_WINDOWS_ACTIVE_KEY and ANALYTICS_WINDOWS_CLEANUP_KEY (moved to analytics SDK) |
core-web/libs/sdk/react/src/lib/next/hooks/useIsAnalyticsActive.ts | Deleted entire hook file (76 lines) - no longer needed for conditional rendering |
core-web/libs/sdk/react/src/lib/next/components/Contentlet/Contentlet.tsx | Simplified component by removing analytics state checks and always applying UVE attributes; added CONTENTLET_CLASS constant |
core-web/libs/sdk/react/src/lib/next/__test__/components/Contentlet.test.tsx | Updated tests to reflect simplified logic - removed analytics mocks and updated assertions |
core-web/libs/sdk/analytics/src/lib/core/shared/constants/dot-analytics.constants.ts | Added ANALYTICS_WINDOWS_ACTIVE_KEY, ANALYTICS_WINDOWS_CLEANUP_KEY, and CONTENTLET_CLASS constants |
core-web/libs/sdk/analytics/src/lib/core/shared/utils/dot-analytics.utils.ts | Updated attribute reading to use standard data-dot-* names instead of data-dot-analytics-* |
core-web/libs/sdk/analytics/src/lib/core/plugin/impression/dot-analytics.impression-tracker.ts | Updated to use data-dot-dom-index instead of data-dot-analytics-dom-index |
core-web/libs/sdk/analytics/src/lib/core/plugin/impression/dot-analytics.impression-tracker.spec.ts | Updated test fixtures to use new attribute names and CONTENTLET_CLASS |
core-web/libs/sdk/analytics/src/lib/core/plugin/identity/dot-analytics.identity.activity-tracker.ts | Updated imports to reference constants from analytics SDK instead of UVE |
core-web/libs/sdk/analytics/src/lib/core/plugin/identity/dot-analytics.identity.activity-tracker.spec.ts | Updated imports to reference constants from analytics SDK instead of UVE |
core-web/libs/sdk/analytics/src/lib/core/dot-analytics.content.ts | Updated imports to reference constants from analytics SDK instead of UVE |
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
| * Window property key for analytics active state | ||
| * Used to track if analytics is initialized and active | ||
| */ | ||
| export const ANALYTICS_CONTENTLET_CLASS = 'dotcms-analytics-contentlet'; | ||
| export const ANALYTICS_WINDOWS_ACTIVE_KEY = '__dotAnalyticsActive__'; | ||
| | ||
| /** | ||
| * Window property key for analytics cleanup function | ||
| * Used to store the cleanup function for analytics instance | ||
| */ | ||
| export const ANALYTICS_WINDOWS_CLEANUP_KEY = '__dotAnalyticsCleanup__'; | ||
| | ||
| /** | ||
| * CSS class selector for contentlet elements | ||
| */ | ||
| export const CONTENTLET_CLASS = 'dotcms-contentlet'; |
There was a problem hiding this comment.
[nitpick] The newly added constants ANALYTICS_WINDOWS_ACTIVE_KEY, ANALYTICS_WINDOWS_CLEANUP_KEY, and CONTENTLET_CLASS are not exported from the analytics SDK's public API (core-web/libs/sdk/analytics/src/lib/react/public-api.ts).
While these constants are currently only used internally within the analytics SDK, the CONTENTLET_CLASS constant is duplicated in the React SDK (core-web/libs/sdk/react/src/lib/next/components/Contentlet/Contentlet.tsx).
Consider either:
- Exporting
CONTENTLET_CLASSfrom the analytics SDK's public API and having the React SDK import it (establishing a single source of truth) - Documenting why this duplication is acceptable (e.g., to avoid cross-package dependencies)
| return classes.length > 0 ? classes.join(' ') : undefined; | ||
| }, [isAnalyticsActive, isDevMode]); | ||
| // UVE attributes - always applied | ||
| const dotAttributes = getDotContentletAttributes(contentlet, container); |
There was a problem hiding this comment.
[nitpick] The getDotContentletAttributes function is now called on every render without memoization. While the function is lightweight (just creates an object), the previous implementation wrapped it in useMemo to avoid recreating the attributes object on every render.
Consider wrapping it back in useMemo for consistency with React best practices and to avoid unnecessary object recreations:
const dotAttributes = useMemo( () => getDotContentletAttributes(contentlet, container), [contentlet, container] );This is especially relevant if contentlet data or containers don't change frequently.
| const dotAttributes = getDotContentletAttributes(contentlet, container); | |
| const dotAttributes = useMemo( | |
| () => getDotContentletAttributes(contentlet, container), | |
| [contentlet, container] | |
| ); |
nicobytes left a comment
nicobytes left a comment There was a problem hiding this comment.
The DotAnalyticsAttributes type definition still references the old attribute naming convention:
type AnalyticsAttribute<T extends string> = `data-dot-analytics-${T}`; export type DotAnalyticsAttributes = { [K in AnalyticsAttributeKey as AnalyticsAttribute<K>]: string; };
| In
|
Summary
Consolidates analytics data attributes by moving constants from UVE to sdk-analytics and simplifies the sdk-react
Contentletcomponent by removing redundant analytics state management. Analytics attributes now always render using UVE's standard contentlet attributes instead of separate conditional logic.Changes Made
SDK Analytics (
libs/sdk/analytics)Moved analytics constants from
@dotcms/uve/internaltosdk-analytics/constants:ANALYTICS_WINDOWS_ACTIVE_KEY- Analytics active state flagANALYTICS_WINDOWS_CLEANUP_KEY- Analytics cleanup function referenceCONTENTLET_CLASS- Renamed fromANALYTICS_CONTENTLET_CLASStodotcms-contentletUpdated data attribute naming to use standard UVE attributes:
data-dot-analytics-identifier→data-dot-identifierdata-dot-analytics-inode→data-dot-inodedata-dot-analytics-contenttype→data-dot-typedata-dot-analytics-title→data-dot-titledata-dot-analytics-basetype→data-dot-basetypedata-dot-analytics-dom-index→data-dot-dom-indexUpdated all analytics plugins to use consolidated constants and new attribute names:
SDK React (
libs/sdk/react)Removed
useIsAnalyticsActivehook (useIsAnalyticsActive.ts)Simplified
Contentletcomponent (Contentlet.tsx:55-78):analyticsAttributesuseMemocontainerClassNameuseMemoisAnalyticsActivestate checkgetDotContentletAttributes()CONTENTLET_CLASS(dotcms-contentlet)Updated tests (Contentlet.test.tsx):
useIsAnalyticsActivemockgetDotAnalyticsAttributesmockSDK UVE (
libs/sdk/uve)getDotAnalyticsAttributes()function (analytics now uses standard UVE attributes)isAnalyticsActive()function (no longer needed)ANALYTICS_WINDOWS_ACTIVE_KEYandANALYTICS_WINDOWS_CLEANUP_KEYconstants (moved to sdk-analytics)Technical Details
Problem: The sdk-react
Contentletcomponent had duplicate analytics attribute logic that conditionally rendered based onisAnalyticsActive && !isDevMode. This created complexity and separated analytics attributes from UVE's standard contentlet attributes.Solution:
@dotcms/analyticswhere they belonggetDotContentletAttributes()which already provides all necessary data attributesdata-dot-*attributesdotcms-contentlet(used by both UVE and analytics)Benefits:
Breaking Changes
CSS Class Change: The analytics contentlet class changed from
dotcms-analytics-contentlettodotcms-contentlet. This aligns with UVE's standard class and should not affect existing implementations as both classes serve the same selector purpose.Data Attribute Changes: Analytics data attributes now use standard UVE naming (
data-dot-*instead ofdata-dot-analytics-*). The analytics SDK internally handles reading these attributes, so no client-side changes are needed.Testing
ContentletcomponentRelated Issues
Closes #33947
Additional Notes
This refactoring aligns with the strategy to consolidate analytics functionality and reduce code duplication across the SDK. Future enhancements can build on this simplified foundation without managing separate analytics attribute logic.
The change ensures analytics tracking is always active and consistent, removing edge cases where analytics might be disabled outside Edit Mode.