Skip to content

chore: Provide additinal component metadata #3483

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
May 12, 2025
Merged

chore: Provide additinal component metadata #3483

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/annotation-context/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react'

import { HotspotProps } from '../hotspot/interfaces';
import { fireNonCancelableEvent } from '../internal/events';
import { useTelemetry } from '../internal/hooks/use-telemetry';
import useBaseComponent from '../internal/hooks/use-base-component';
import { applyDisplayName } from '../internal/utils/apply-display-name';
import { ClosedAnnotation } from './annotation/closed-annotation';
import { OpenAnnotation } from './annotation/open-annotation';
Expand All @@ -26,7 +26,7 @@ export default function AnnotationContext({
onExitTutorial,
i18nStrings,
}: AnnotationContextProps): JSX.Element {
useTelemetry('AnnotationContext');
useBaseComponent('AnnotationContext');
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switched all non-UI components to useBaseComponent for consistency.

The only difference, there is __internalRootRef which is left unused in these components, but the ref handling code checks its presence anyway, so this ref is effectively a no-op


const [open, setOpen] = useState(true);

Expand Down
4 changes: 2 additions & 2 deletions src/i18n/provider.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import IntlMessageFormat from 'intl-messageformat';

import { warnOnce } from '@cloudscape-design/component-toolkit/internal';

import { useTelemetry } from '../internal/hooks/use-telemetry';
import useBaseComponent from '../internal/hooks/use-base-component';
import { applyDisplayName } from '../internal/utils/apply-display-name';
import { CustomHandler, FormatFunction, InternalI18nContext } from './context';
import { getMatchableLocales } from './get-matchable-locales';
Expand Down Expand Up @@ -38,7 +38,7 @@ export namespace I18nProviderProps {
const I18nMessagesContext = React.createContext<I18nProviderProps.Messages>({});

export function I18nProvider({ messages: messagesArray, locale: providedLocale, children }: I18nProviderProps) {
useTelemetry('I18nProvider');
useBaseComponent('I18nProvider');

if (typeof document === 'undefined' && !providedLocale) {
warnOnce(
Expand Down
16 changes: 11 additions & 5 deletions src/internal/hooks/use-base-component/__tests__/use-base-component.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,20 @@
import React, { useState } from 'react';
import { render } from '@testing-library/react';

import { COMPONENT_METADATA_KEY, Portal } from '@cloudscape-design/component-toolkit/internal';
import { COMPONENT_METADATA_KEY, Portal, useComponentMetrics } from '@cloudscape-design/component-toolkit/internal';

import { Button } from '../../../../../lib/components';
import { PACKAGE_VERSION } from '../../../../../lib/components/internal/environment';
import useBaseComponent, {
InternalBaseComponentProps,
} from '../../../../../lib/components/internal/hooks/use-base-component';
import { useTelemetry } from '../../../../../lib/components/internal/hooks/use-telemetry';
import createWrapper from '../../../../../lib/components/test-utils/dom';

jest.mock('../../../../../lib/components/internal/hooks/use-telemetry', () => {
return { useTelemetry: jest.fn(() => null) };
jest.mock('@cloudscape-design/component-toolkit/internal', () => {
return {
...jest.requireActual('@cloudscape-design/component-toolkit/internal'),
useComponentMetrics: jest.fn(() => {}),
};
});

type InternalDemoProps = InternalBaseComponentProps;
Expand Down Expand Up @@ -62,7 +64,11 @@ test('should attach the metadata to the returned root DOM node', () => {
test('should call the useTelemetry hook passing down the given component name and its props', () => {
jest.resetAllMocks();
render(<Demo variant="default" />);
expect(useTelemetry).toHaveBeenCalledWith('DemoComponent', { props: { variant: 'default' } });
expect(useComponentMetrics).toHaveBeenCalledWith(
'DemoComponent',
expect.objectContaining({ packageVersion: PACKAGE_VERSION }),
{ props: { variant: 'default' } }
);
});

test('metadata get attached on the Portal component root DOM node when elementRef is changing', () => {
Expand Down
16 changes: 12 additions & 4 deletions src/internal/hooks/use-base-component/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@ import { MutableRefObject } from 'react';
import {
ComponentConfiguration,
useComponentMetadata,
useComponentMetrics,
useFocusVisible,
} from '@cloudscape-design/component-toolkit/internal';

import { AnalyticsMetadata } from '../../analytics/interfaces';
import { PACKAGE_VERSION } from '../../environment';
import { useTelemetry } from '../use-telemetry';
import { PACKAGE_SOURCE, PACKAGE_VERSION, THEME } from '../../environment';
import { getVisualTheme } from '../../utils/get-visual-theme';
import { useVisualRefresh } from '../use-visual-mode';

export interface InternalBaseComponentProps<T = any> {
__internalRootRef?: MutableRefObject<T | null> | null;
Expand All @@ -26,8 +28,14 @@ export default function useBaseComponent<T = any>(
config?: ComponentConfiguration,
analyticsMetadata?: AnalyticsMetadata
) {
useTelemetry(componentName, config);
const isVisualRefresh = useVisualRefresh();
const theme = getVisualTheme(THEME, isVisualRefresh);
useComponentMetrics(componentName, { packageSource: PACKAGE_SOURCE, packageVersion: PACKAGE_VERSION, theme }, config);
useFocusVisible();
const elementRef = useComponentMetadata<T>(componentName, PACKAGE_VERSION, { ...analyticsMetadata });
const elementRef = useComponentMetadata<T>(
componentName,
{ packageName: PACKAGE_SOURCE, version: PACKAGE_VERSION, theme },
Comment on lines +33 to +37
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is an annoying structure difference between useComponentMetrics and useComponentMetadata. They receive same data, but in different shape. Still some refactoring needed.

analyticsMetadata as any
Copy link
Member Author

@just-boris just-boris May 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This value is typed differently in the toolkit and this package

// this package
export interface AnalyticsMetadata {
  instanceIdentifier?: string;
  flowType?: FlowType;
  errorContext?: string;
  resourceType?: string;
}

// toolkit
export type AnalyticsMetadata = JSONObject;

This component's more strict type is not assignable to the toolkit. Before my change we used to fool typescript with runtime code { ...analyticsMetadata }. I do not like it. If we mess with types anyway, let's do it more clearly at least

);
return { __internalRootRef: elementRef };
}
13 changes: 0 additions & 13 deletions src/internal/hooks/use-telemetry/index.ts
View file
Open in desktop

This file was deleted.

Loading