Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 8 additions & 5 deletions src/core/PaperProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { getDefaultDirection, LocaleProvider, type Direction } from './locale';
import SafeAreaProviderCompat from './SafeAreaProviderCompat';
import { Provider as SettingsProvider } from './settings';
import type { Settings } from './settings';
import { defaultThemes, ThemeProvider } from './theming';
import { defaultThemes, safeMerge, ThemeProvider } from './theming';
import {
useResolvedReduceMotion,
type ReduceMotionPreference,
Expand Down Expand Up @@ -36,11 +36,14 @@ const PaperProvider = (props: Props) => {
? 0
: (props.theme?.animation?.scale ?? 1);

// Deep merge so a partial theme extends the defaults instead of replacing
// them. `safeMerge` is the same helper `useInternalTheme` uses for the
// per-component `theme` prop, which keeps both levels consistent.
const merged = safeMerge(base, props.theme);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

calling safeMerge at the theme root can drop all defaults when a custom theme contains dynamic, semantic or resource_paths

current sentinel check treats any object containing one of these keys as a native color, while custom theme properties are supported

safeMerge(base, { dynamic: true });
// => { dynamic: true }, dropping fonts, colors, shapes etc.

RN’s actual native-color values have narrower shapes, such as { semantic: string[] } or { dynamic: { light, dark } } (source)

so could we validate those shapes before using safeMerge here & add regression test for custom dynamic property?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Good catch — you're right, and this is a real regression introduced by moving safeMerge to the theme root. Fixed in 25363ae.

Reproduced first: safeMerge(base, { dynamic: true }) returned { dynamic: true }, and <PaperProvider theme={{ dynamic: true }}> made <Text variant="titleLarge"> throw again — the exact symptom this PR set out to fix.

isPlatformColorSentinel now validates the value, not just the key name. Checking the shapes React Native actually emits (PlatformColorValueTypes.ios.js L25-28 and L37-50, PlatformColorValueTypes.android.js L19-23), a native color value carries exactly one key and nothing else, so the check now accepts only:

  • { semantic: string[] } — iOS PlatformColor
  • { resource_paths: string[] } — Android PlatformColor
  • { dynamic: { light, dark, highContrastLight?, highContrastDark? } }DynamicColorIOS, requiring both light and dark and rejecting any key outside that tuple

Everything else — including { dynamic: true }, { semantic: 'label' }, and any object that carries a sentinel key alongside siblings such as colors/fonts — is now an ordinary object and gets deep-merged, so a documented custom theme property called dynamic keeps every default.

Regression tests added, as requested:

  • PaperProvider.test.tsx: a theme { dynamic: true, colors: { primary: 'tomato' } } keeps the typescale, palette and shape tokens, plus a user-visible one asserting <Text variant="titleLarge"> renders under theme={{ dynamic: true }}.
  • provider.test.ts: rejection cases for the three key names with wrong-shaped values and with sibling keys.

Two of the new tests are deliberately non-vacuous controls that pass both before and after, so the tightening can't quietly degenerate into "nothing is ever a sentinel": one asserts real PlatformColor('label') and DynamicColorIOS(...) values are still detected, and one asserts a real DynamicColorIOS override still passes through safeMerge by identity, with no highContrast* keys inherited from the base sentinel underneath it. With the sentinel change reverted the six new tests fail and those two still pass.

Suite: 55 suites / 737 → 745 passed / 1 skipped, 169 snapshots unchanged; lint and typecheck clean.


return {
...base,
...props.theme,
colors: { ...base.colors, ...props.theme?.colors },
animation: { ...props.theme?.animation, scale },
...merged,
animation: { ...merged.animation, scale },
};
}, [colorScheme, props.theme, resolvedReduceMotion]);

Expand Down
122 changes: 122 additions & 0 deletions src/core/__tests__/PaperProvider.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
} from '@jest/globals';
import { act, render, screen } from '@testing-library/react-native';

import Text from '../../components/Typography/Text';
import { useReduceMotion } from '../../theme/accessibility/ReduceMotionContext';
import { DarkTheme, DynamicLightTheme, LightTheme } from '../../theme/schemes';
import type { ThemeProp } from '../../types';
Expand Down Expand Up @@ -329,4 +330,125 @@ describe('PaperProvider', () => {
customTheme
);
});

describe('partial theme merging', () => {
// A v2-shaped `fonts` object, as produced by `configureFonts` before v5.13
// and still widely copy-pasted. It shares no keys with the MD3 typescale.
const legacyFonts = {
regular: { fontFamily: 'CustomSans-Regular', fontWeight: '400' },
medium: { fontFamily: 'CustomSans-Medium', fontWeight: '500' },
light: { fontFamily: 'CustomSans-Light', fontWeight: '300' },
thin: { fontFamily: 'CustomSans-Thin', fontWeight: '100' },
} as const;

it('keeps the base typescale when only part of theme.fonts is provided', async () => {
mockAppearance();
await render(createProvider({ fonts: legacyFonts } as ThemeProp));

const theme =
// eslint-disable-next-line no-restricted-syntax -- TODO: replace TestInstance props access with a user-visible assertion.
screen.getByTestId('provider-child-view').props.theme;

// The MD3 variants the user did not mention must survive...
expect(theme.fonts.titleLarge).toStrictEqual(LightTheme.fonts.titleLarge);
expect(theme.fonts.bodyMedium).toStrictEqual(LightTheme.fonts.bodyMedium);
// ...alongside the keys the user did provide.
expect(theme.fonts.regular).toStrictEqual(legacyFonts.regular);
});

it('renders <Text variant> instead of throwing when theme.fonts is partial', async () => {
mockAppearance();
// Reproduces #4589: `<Text variant>` threw
// "Variant titleLarge was not provided properly. Valid variants are
// regular, medium, light, thin." because the provider dropped the typescale.
await render(
<PaperProvider theme={{ fonts: legacyFonts } as ThemeProp}>
<Text variant="titleLarge">Merged typescale</Text>
</PaperProvider>
);

expect(screen.getByText('Merged typescale')).toBeOnTheScreen();
});

it('still merges theme.colors with the base palette', async () => {
mockAppearance();
await render(createProvider({ colors: { primary: 'tomato' } }));

const theme =
// eslint-disable-next-line no-restricted-syntax -- TODO: replace TestInstance props access with a user-visible assertion.
screen.getByTestId('provider-child-view').props.theme;

expect(theme.colors.primary).toBe('tomato');
expect(theme.colors.onSurface).toBe(LightTheme.colors.onSurface);
expect(Object.keys(theme.colors)).toStrictEqual(
Object.keys(LightTheme.colors)
);
});

it('keeps sibling tokens when a nested shape token is overridden', async () => {
mockAppearance();
await render(createProvider({ shapes: { corner: { small: 2 } } }));

const theme =
// eslint-disable-next-line no-restricted-syntax -- TODO: replace TestInstance props access with a user-visible assertion.
screen.getByTestId('provider-child-view').props.theme;

expect(theme.shapes.corner.small).toBe(2);
expect(theme.shapes.corner.large).toBe(LightTheme.shapes.corner.large);
});

it('keeps the defaults when the theme owns a custom property named `dynamic`', async () => {
mockAppearance();
// `dynamic`, `semantic` and `resource_paths` are the keys that mark a
// native platform color. A user theme is allowed to own them as ordinary
// custom properties (docs: "Extending the theme"), and doing so must not
// make the whole theme look like a leaf value.
await render(
createProvider({
dynamic: true,
colors: { primary: 'tomato' },
} as ThemeProp)
);

const theme =
// eslint-disable-next-line no-restricted-syntax -- TODO: replace TestInstance props access with a user-visible assertion.
screen.getByTestId('provider-child-view').props.theme;

expect(theme.dynamic).toBe(true);
expect(theme.colors.primary).toBe('tomato');
expect(theme.colors.onSurface).toBe(LightTheme.colors.onSurface);
expect(theme.fonts.titleLarge).toStrictEqual(LightTheme.fonts.titleLarge);
expect(theme.shapes.corner.large).toBe(LightTheme.shapes.corner.large);
});

it('renders <Text variant> when the theme owns a custom `dynamic` property', async () => {
mockAppearance();
await render(
<PaperProvider theme={{ dynamic: true } as ThemeProp}>
<Text variant="titleLarge">Custom dynamic property</Text>
</PaperProvider>
);

expect(screen.getByText('Custom dynamic property')).toBeOnTheScreen();
});

it('lets a complete fonts object override every default variant', async () => {
mockAppearance();
// Shaped like the output of `configureFonts`: every variant present, with
// the same properties as the defaults, so nothing can be inherited.
const completeFonts = Object.fromEntries(
Object.entries(LightTheme.fonts).map(([variant, style]) => [
variant,
{ ...style, fontFamily: 'Overridden' },
])
);
await render(createProvider({ fonts: completeFonts } as ThemeProp));

const theme =
// eslint-disable-next-line no-restricted-syntax -- TODO: replace TestInstance props access with a user-visible assertion.
screen.getByTestId('provider-child-view').props.theme;

expect(theme.fonts).toStrictEqual(completeFonts);
});
});
});
99 changes: 99 additions & 0 deletions src/theme/__tests__/provider.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,14 @@
import { DynamicColorIOS, PlatformColor } from 'react-native';

import { describe, expect, it } from '@jest/globals';

import { isPlatformColorSentinel, safeMerge } from '../provider';

// Android's `PlatformColor` cannot be exercised here (jest resolves the `.ios`
// platform extension), so its value is spelled out. Shape taken verbatim from
// react-native/Libraries/StyleSheet/PlatformColorValueTypes.android.js.
const androidPlatformColor = { resource_paths: ['@android:color/black'] };

describe('isPlatformColorSentinel', () => {
it('detects iOS PlatformColor (semantic)', () => {
expect(isPlatformColorSentinel({ semantic: ['label'] })).toBe(true);
Expand All @@ -19,6 +26,62 @@ describe('isPlatformColorSentinel', () => {
).toBe(true);
});

it('detects values produced by the real react-native APIs', () => {
// Guards against the shape validation below degenerating into
// "nothing is ever a sentinel", which would let deepmerge corrupt
// genuine platform colors again.
expect(isPlatformColorSentinel(PlatformColor('label'))).toBe(true);
expect(
isPlatformColorSentinel(DynamicColorIOS({ light: '#fff', dark: '#000' }))
).toBe(true);
expect(
isPlatformColorSentinel(
DynamicColorIOS({
light: '#fff',
dark: '#000',
highContrastLight: '#eee',
highContrastDark: '#111',
})
)
).toBe(true);
expect(isPlatformColorSentinel(androidPlatformColor)).toBe(true);
});

it('rejects custom theme properties that only reuse a sentinel key name', () => {
// Extending the theme with arbitrary properties is documented, so a theme
// is allowed to own a key called `dynamic`, `semantic` or `resource_paths`.
expect(isPlatformColorSentinel({ dynamic: true })).toBe(false);
expect(isPlatformColorSentinel({ dynamic: 'auto' })).toBe(false);
expect(isPlatformColorSentinel({ semantic: 'label' })).toBe(false);
expect(isPlatformColorSentinel({ semantic: [1, 2] })).toBe(false);
expect(isPlatformColorSentinel({ resource_paths: true })).toBe(false);
});

it('rejects objects that carry a sentinel key alongside other keys', () => {
// A whole theme is not a platform color, even when one of its properties
// happens to be shaped like `DynamicColorIOS`'s tuple.
expect(
isPlatformColorSentinel({
dynamic: { light: '#fff', dark: '#000' },
colors: { primary: 'tomato' },
})
).toBe(false);
expect(isPlatformColorSentinel({ semantic: ['label'], fonts: {} })).toBe(
false
);
});

it('rejects `dynamic` values that are not a light/dark tuple', () => {
expect(isPlatformColorSentinel({ dynamic: {} })).toBe(false);
expect(isPlatformColorSentinel({ dynamic: { light: '#fff' } })).toBe(false);
expect(isPlatformColorSentinel({ dynamic: { dark: '#000' } })).toBe(false);
expect(
isPlatformColorSentinel({
dynamic: { light: '#fff', dark: '#000', scale: 1 },
})
).toBe(false);
});

it('rejects plain objects, primitives, null, and arrays', () => {
expect(isPlatformColorSentinel({ primary: '#fff' })).toBe(false);
expect(isPlatformColorSentinel('#fff')).toBe(false);
Expand Down Expand Up @@ -98,6 +161,42 @@ describe('safeMerge', () => {
expect(result.colors.primary).toBe(sentinelOverride);
});

it('keeps the base when overrides own a custom property named `dynamic`', () => {
const base = {
fonts: { titleLarge: { fontSize: 22 } },
colors: { primary: '#000' },
};
const overrides = { dynamic: true };

const result = safeMerge<typeof base & { dynamic?: boolean }>(
base,
overrides
);

expect(result.fonts).toStrictEqual(base.fonts);
expect(result.colors).toStrictEqual(base.colors);
expect(result.dynamic).toBe(true);
});

it('still treats a real DynamicColorIOS override as a leaf, not a merge target', () => {
const baseColor = DynamicColorIOS({
light: '#000',
dark: '#111',
highContrastLight: '#222',
highContrastDark: '#333',
});
const overrideColor = DynamicColorIOS({ light: '#fff', dark: '#eee' });
const base = { colors: { primary: baseColor } };
const overrides = { colors: { primary: overrideColor } };

const result = safeMerge<{ colors: { primary: any } }>(base, overrides);

// Identity: the override object is passed through untouched...
expect(result.colors.primary).toBe(overrideColor);
// ...and nothing was inherited from the base sentinel underneath it.
expect(result.colors.primary.dynamic.highContrastLight).toBeUndefined();
});

it('preserves sentinel siblings when merging a colors map', () => {
const sentinel = { semantic: ['label'] };
const base = {
Expand Down
64 changes: 56 additions & 8 deletions src/theme/provider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,63 @@ export function useTheme<T = Theme>(overrides?: $DeepPartial<T>) {
return useThemeBase<T>(overrides);
}

const isStringArray = (v: unknown): boolean =>
Array.isArray(v) && v.every((item) => typeof item === 'string');

const DYNAMIC_TUPLE_KEYS = [
'light',
'dark',
'highContrastLight',
'highContrastDark',
];

// `DynamicColorIOS` always emits both `light` and `dark` (either may be
// nullish) and never any key outside the tuple above.
const isDynamicColorIOSTuple = (v: unknown): boolean => {
if (!v || typeof v !== 'object' || Array.isArray(v)) {
return false;
}
const keys = Object.keys(v);
return (
keys.includes('light') &&
keys.includes('dark') &&
keys.every((key) => DYNAMIC_TUPLE_KEYS.includes(key))
);
};

// Upstream `deepmerge` corrupts PlatformColor objects, so we recurse manually
// and treat sentinels as leaves. Three shapes:
// `semantic` — iOS PlatformColor
// `dynamic` — DynamicColorIOS
// `resource_paths` — Android PlatformColor
export const isPlatformColorSentinel = (v: unknown): boolean =>
!!v &&
typeof v === 'object' &&
('resource_paths' in v || 'semantic' in v || 'dynamic' in v);
// and treat sentinels as leaves. Three shapes, straight from React Native's
// `PlatformColorValueTypes.{ios,android}.js`:
// `{ semantic: string[] }` — iOS PlatformColor
// `{ dynamic: { light, dark, ...} }` — DynamicColorIOS
// `{ resource_paths: string[] }` — Android PlatformColor
// The shape has to be validated, not just the key name: a theme may own a
// custom property called `dynamic`, `semantic` or `resource_paths` (extending
// the theme with arbitrary properties is documented), and treating such a
// theme as a leaf would drop every default it did not spell out.
export const isPlatformColorSentinel = (v: unknown): boolean => {
if (!v || typeof v !== 'object' || Array.isArray(v)) {
return false;
}
// A native color value carries exactly one of the three keys and nothing
// else, so anything with siblings is a regular object.
const keys = Object.keys(v);
if (keys.length !== 1) {
return false;
}
const [key] = keys;
const value = (v as Record<string, unknown>)[key];

switch (key) {
case 'semantic':
case 'resource_paths':
return isStringArray(value);
case 'dynamic':
return isDynamicColorIOSTuple(value);
default:
return false;
}
};

export const safeMerge = <T,>(base: T, overrides: unknown): T => {
if (
Expand Down