Skip to content

[FIX] Codepush: unset 'useAccessibilityLabel' and 'actionNameAttribute' properties #890

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
marco-saia-datadog merged 1 commit into from
May 29, 2025
Merged

[FIX] Codepush: unset 'useAccessibilityLabel' and 'actionNameAttribute' properties #890

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
2 changes: 1 addition & 1 deletion packages/codepush/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
"prepare": "rm -rf lib && yarn bob build"
},
"devDependencies": {
"@datadog/mobile-react-native": "^2.8.0",
"@datadog/mobile-react-native": "workspace:packages/core",
"@testing-library/react-native": "7.0.2",
"react-native-builder-bob": "0.26.0",
"react-native-code-push": "7.1.0"
Expand Down
196 changes: 187 additions & 9 deletions packages/codepush/src/__tests__/index.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,22 +1,19 @@
import {
DdSdkReactNative,
DdSdkReactNativeConfiguration,
DatadogProviderConfiguration
} from '@datadog/mobile-react-native';
import { render } from '@testing-library/react-native';
import codePush from 'react-native-code-push';
/* eslint-disable @typescript-eslint/no-var-requires */
/* eslint-disable global-require */
import { render, waitFor } from '@testing-library/react-native';
import React from 'react';

import { DatadogCodepush, DatadogCodepushProvider } from '..';

jest.mock('react-native-code-push', () => ({
getUpdateMetadata: jest.fn()
}));

jest.mock('@datadog/mobile-react-native', () => {
const actualPackage = jest.requireActual('@datadog/mobile-react-native');
actualPackage.DdSdkReactNative.initialize = jest.fn();
actualPackage.DdSdkReactNative._enableFeaturesFromDatadogProvider = jest.fn();
actualPackage.DdSdkReactNative._enableFeaturesFromDatadogProviderAsync = jest.fn();
actualPackage.DdSdkReactNative._initializeFromDatadogProviderWithConfigurationAsync = jest.fn();
actualPackage.DdSdkReactNative._initializeFromDatadogProvider = jest.fn();
return actualPackage;
});

Expand All @@ -41,8 +38,16 @@ describe('AppCenter Codepush integration', () => {
beforeEach(() => {
jest.clearAllMocks();
});

describe('initialize', () => {
it('initializes the SDK with the correct version when using a CodePush bundle', async () => {
const codePush = require('react-native-code-push');
const { DatadogCodepush } = require('..');
Comment thread
marco-saia-datadog marked this conversation as resolved.
const {
DdSdkReactNativeConfiguration,
DdSdkReactNative
} = require('@datadog/mobile-react-native');

(codePush.getUpdateMetadata as jest.MockedFunction<
typeof codePush.getUpdateMetadata
>).mockResolvedValueOnce(createCodepushPackageMock('v3'));
Expand All @@ -65,6 +70,13 @@ describe('AppCenter Codepush integration', () => {
});

it('initializes the SDK with the correct version when not using a CodePush bundle', async () => {
const codePush = require('react-native-code-push');
const { DatadogCodepush } = require('..');
Comment thread
marco-saia-datadog marked this conversation as resolved.
const {
DdSdkReactNativeConfiguration,
DdSdkReactNative
} = require('@datadog/mobile-react-native');

(codePush.getUpdateMetadata as jest.MockedFunction<
typeof codePush.getUpdateMetadata
>).mockResolvedValueOnce(null);
Expand Down Expand Up @@ -92,7 +104,19 @@ describe('AppCenter Codepush integration', () => {
});

describe('DatadogCodepushProvider', () => {
beforeEach(() => {
jest.clearAllMocks();
jest.resetModules();
});

it('initializes the sdk with the right codepush version when using DatadogProviderConfiguration', async () => {
const codePush = require('react-native-code-push');
Comment thread
marco-saia-datadog marked this conversation as resolved.
const { DatadogCodepushProvider } = require('..');
const {
DatadogProviderConfiguration,
DdSdkReactNative
} = require('@datadog/mobile-react-native');

(codePush.getUpdateMetadata as jest.MockedFunction<
typeof codePush.getUpdateMetadata
>).mockResolvedValueOnce(createCodepushPackageMock('v4'));
Expand All @@ -118,6 +142,12 @@ describe('AppCenter Codepush integration', () => {
);
});
it('initializes the sdk with the right codepush version when using partial configuration', async () => {
const codePush = require('react-native-code-push');
const { DatadogCodepushProvider } = require('..');
const {
DdSdkReactNative
} = require('@datadog/mobile-react-native');
Comment thread
marco-saia-datadog marked this conversation as resolved.

(codePush.getUpdateMetadata as jest.MockedFunction<
typeof codePush.getUpdateMetadata
>).mockResolvedValueOnce(createCodepushPackageMock('v5'));
Expand Down Expand Up @@ -149,7 +179,15 @@ describe('AppCenter Codepush integration', () => {
expect.objectContaining({ versionSuffix: 'codepush.v5' })
);
});

it('initializes the sdk with commercial version when using DatadogProviderConfiguration', async () => {
const codePush = require('react-native-code-push');
Comment thread
marco-saia-datadog marked this conversation as resolved.
const { DatadogCodepushProvider } = require('..');
const {
DatadogProviderConfiguration,
DdSdkReactNative
} = require('@datadog/mobile-react-native');

(codePush.getUpdateMetadata as jest.MockedFunction<
typeof codePush.getUpdateMetadata
>).mockResolvedValueOnce(createCodepushPackageMock(null));
Expand Down Expand Up @@ -177,6 +215,12 @@ describe('AppCenter Codepush integration', () => {
).not.toContain('versionSuffix');
});
it('initializes the sdk with commercial version when using partial configuration', async () => {
const codePush = require('react-native-code-push');
const { DatadogCodepushProvider } = require('..');
Comment thread
marco-saia-datadog marked this conversation as resolved.
const {
DdSdkReactNative
} = require('@datadog/mobile-react-native');

(codePush.getUpdateMetadata as jest.MockedFunction<
typeof codePush.getUpdateMetadata
>).mockResolvedValueOnce(createCodepushPackageMock(null));
Expand Down Expand Up @@ -210,5 +254,139 @@ describe('AppCenter Codepush integration', () => {
)
).not.toContain('versionSuffix');
});

it('initializes the DatadogProvider with FileBasedConfiguration & all parameters', async () => {
const { DatadogCodepushProvider } = require('..');
Comment thread
marco-saia-datadog marked this conversation as resolved.
const {
DdSdkReactNative,
PropagatorType,
FileBasedConfiguration
} = require('@datadog/mobile-react-native');

const autoInstrumentationConfig = {
trackErrors: true,
trackResources: true,
trackInteractions: true,
firstPartyHosts: [
{
match: 'example.com',
propagatorTypes: [PropagatorType.DATADOG]
}
],
useAccessibilityLabel: true,
actionNameAttribute: 'test-action-name-attr',
resourceTracingSamplingRate: 100
};

const configuration = new FileBasedConfiguration({
configuration: { configuration: autoInstrumentationConfig }
});

render(<DatadogCodepushProvider configuration={configuration} />);

await flushPromises();
await waitFor(() => {
expect(
DdSdkReactNative._enableFeaturesFromDatadogProvider
).toHaveBeenCalledTimes(1);
});
expect(
DdSdkReactNative._enableFeaturesFromDatadogProvider
).toHaveBeenCalledWith({
actionEventMapper: null,
logEventMapper: null,
resourceEventMapper: null,
errorEventMapper: null,
trackErrors: true,
trackResources: true,
trackInteractions: true,
firstPartyHosts: [
{
match: 'example.com',
propagatorTypes: [PropagatorType.DATADOG]
}
],
useAccessibilityLabel: true,
actionNameAttribute: 'test-action-name-attr',
resourceTracingSamplingRate: 100
});

expect(
DdSdkReactNative._enableFeaturesFromDatadogProvider
).not.toHaveBeenCalledWith(
expect.objectContaining({
clientToken: expect.anything(),
env: expect.anything(),
applicationId: expect.anything()
})
);
});

it('initializes the DatadogProvider with FileBasedConfiguration & undefined parameters', async () => {
const { DatadogCodepushProvider } = require('..');
const {
DdSdkReactNative,
PropagatorType,
FileBasedConfiguration
} = require('@datadog/mobile-react-native');

const autoInstrumentationConfig = {
trackErrors: true,
trackResources: true,
trackInteractions: true,
firstPartyHosts: [
{
match: 'example.com',
propagatorTypes: [PropagatorType.DATADOG]
}
],
// useAccessibilityLabel: true,
// actionNameAttribute: 'test-action-name-attr',
resourceTracingSamplingRate: 100
};

const configuration = new FileBasedConfiguration({
configuration: { configuration: autoInstrumentationConfig }
});

render(<DatadogCodepushProvider configuration={configuration} />);

await flushPromises();
await waitFor(() => {
expect(
DdSdkReactNative._enableFeaturesFromDatadogProvider
).toHaveBeenCalledTimes(1);
});
expect(
DdSdkReactNative._enableFeaturesFromDatadogProvider
).toHaveBeenCalledWith({
actionEventMapper: null,
logEventMapper: null,
resourceEventMapper: null,
errorEventMapper: null,
trackErrors: true,
trackResources: true,
trackInteractions: true,
firstPartyHosts: [
{
match: 'example.com',
propagatorTypes: [PropagatorType.DATADOG]
}
],
resourceTracingSamplingRate: 100,
actionNameAttribute: undefined,
useAccessibilityLabel: true
});

expect(
DdSdkReactNative._enableFeaturesFromDatadogProvider
).not.toHaveBeenCalledWith(
expect.objectContaining({
clientToken: expect.anything(),
env: expect.anything(),
applicationId: expect.anything()
})
);
});
});
});
46 changes: 36 additions & 10 deletions packages/codepush/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,22 @@
/*
* Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0.
* This product includes software developed at Datadog (https://www.datadoghq.com/).
* Copyright 2016-Present Datadog, Inc.
*/
import {
DatadogProvider,
DatadogProviderConfiguration,
DdSdkReactNative
} from '@datadog/mobile-react-native';
import type { DdSdkReactNativeConfiguration } from '@datadog/mobile-react-native';
import type {
AutoInstrumentationConfiguration,
DdSdkReactNativeConfiguration
} from '@datadog/mobile-react-native';
import codePush from 'react-native-code-push';

import { DISCARD_PROPERTY, removeDiscardProperties } from './utils';
import type { RequiredOrDiscard } from './utils';

/**
* Use this class instead of DdSdkReactNative to initialize the Datadog SDK when using AppCenter CodePush.
*/
Expand All @@ -31,6 +42,29 @@ const initializeWithCodepushVersion = async (
DatadogProvider.initialize(configuration);
};

const buildPartialConfiguration = (
configuration: DatadogProviderConfiguration
): AutoInstrumentationConfiguration => {
const partialConfiguration: RequiredOrDiscard<AutoInstrumentationConfiguration> = {
trackErrors: configuration.trackErrors,
trackResources: configuration.trackResources,
trackInteractions: configuration.trackInteractions,
firstPartyHosts: configuration.firstPartyHosts,
logEventMapper: configuration.logEventMapper,
errorEventMapper: configuration.errorEventMapper,
resourceEventMapper: configuration.resourceEventMapper,
actionEventMapper: configuration.actionEventMapper,
useAccessibilityLabel: configuration.useAccessibilityLabel,
resourceTracingSamplingRate: configuration.resourceTracingSamplingRate,
actionNameAttribute:
configuration.actionNameAttribute ?? DISCARD_PROPERTY
};

return removeDiscardProperties(
partialConfiguration
) as AutoInstrumentationConfiguration;
Comment on lines +48 to +65
Copy link
Copy Markdown
Contributor

@cdn34dd cdn34dd May 29, 2025

Choose a reason for hiding this comment

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

I'm having trouble understanding this, why not just make it partial ?

const partialConfiguration: Partial<AutoInstrumentationConfiguration> ?

or if you want some to be partial and others to be required, maybe doing something like this ?

const partialConfiguration: Partial<AutoInstrumentationConfiguration> & {trackInteractions: AutoInstrumentationConfiguration['trackInteractions']}

But the way we're doing the types here doesn't feel right.

Copy link
Copy Markdown
Member Author

@marco-saia-datadog marco-saia-datadog May 29, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

The problem with Partial is that you are going to miss certain properties, because TSC won't complain.

We can't directly assign each property of DatadogProviderConfiguration to the partial configuration, because we want to skip the extra ones.

If we use Required<AutoInstrumentationConfiguration>, optional properties like actionNameAttribute will require a non-undefined value, and will make TSC complain, which is also not good.

If we use Partial<AutoInstrumentationConfiguration>, and we don't explicitly add the properties if we introduce new ones in the future, we won't catch it with TSC. Partial should be used when certain properties can be skipped, but this is not the case. Optional properties still have to be mapped using the DatadogProviderConfiguration properties.

I know this does not feel right, but I honestly could not find a good alternative to type it in a way that would force developers to map all properties.

};

export const DatadogCodepushProvider: typeof DatadogProvider = ({
configuration,
...rest
Expand All @@ -39,16 +73,8 @@ export const DatadogCodepushProvider: typeof DatadogProvider = ({
// We turn it to partial initialization, while in parallel we get the CodePush version and initialize the SDK.
if (configuration instanceof DatadogProviderConfiguration) {
initializeWithCodepushVersion(configuration);
const partialConfiguration = {
trackErrors: configuration.trackErrors,
trackResources: configuration.trackResources,
trackInteractions: configuration.trackInteractions,
firstPartyHosts: configuration.firstPartyHosts,
resourceTracingSamplingRate:
configuration.resourceTracingSamplingRate
};
return DatadogProvider({
configuration: partialConfiguration,
configuration: buildPartialConfiguration(configuration),
...rest
});
} else {
Expand Down
37 changes: 37 additions & 0 deletions packages/codepush/src/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0.
* This product includes software developed at Datadog (https://www.datadoghq.com/).
* Copyright 2016-Present Datadog, Inc.
*/

/**
* A constant used to define a property that should be discarded from a {@link RequiredOrDiscard} object.
*/
export const DISCARD_PROPERTY = { _dd_meta_: 'DISCARD' };

/**
* Used to change the type of every property of an object to be either required or {@link DISCARD_PROPERTY}.
*/
export type RequiredOrDiscard<T> = {
[K in keyof T]-?: T[K] | typeof DISCARD_PROPERTY;
};

/**
* Removes all entries of value {@link DISCARD_PROPERTY} from the given object
* @param obj The object to remove the {@link DISCARD_PROPERTY} entries from.
* @returns The object without the {@link DISCARD_PROPERTY} entries.
*/
export const removeDiscardProperties = <T extends Record<string, any>>(
Comment thread
marco-saia-datadog marked this conversation as resolved.
obj: T
): {
[K in keyof T]: T[K] extends null ? undefined : T[K];
} => {
const result = {} as any;
Comment thread
marco-saia-datadog marked this conversation as resolved.

Object.keys(obj).forEach(key => {
const value = obj[key];
result[key] = value === DISCARD_PROPERTY ? undefined : value;
});

return result;
};
Loading
Loading