feat: redo FluentProvider theme api (#21286)

* fix(react-provider):make the Provider#theme TS API reflect runtime and add warning if there is a violation * fix(react-shared-contexts): make the ThemeContext TS API reflect runtime and remove redundant ThemeContextValue * Change files * fixup! fix(react-shared-contexts): make the ThemeContext TS API reflect runtime and remove redundant ThemeContextValue * fixup! fix(react-provider):make the Provider#theme TS API reflect runtime and add warning if there is a violation * fixup! fixup! fix(react-provider):make the Provider#theme TS API reflect runtime and add warning if there is a violation
microsoft · Jan 14, 2022 · ecfe4c4 · ecfe4c4
1 parent 1dba2fc
commit ecfe4c4
Show file tree

Hide file tree

Showing 13 changed files with 96 additions and 36 deletions.
diff --git a/change/@fluentui-react-provider-799f4a36-cc2b-48d7-ba65-9b54b83ac24a.json b/change/@fluentui-react-provider-799f4a36-cc2b-48d7-ba65-9b54b83ac24a.json
@@ -0,0 +1,7 @@
+{
+  "type": "prerelease",
+  "comment": "make the FluentProvider#theme TS API reflect runtime and add warning if there is a violation",
+  "packageName": "@fluentui/react-provider",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}
diff --git a/change/@fluentui-react-shared-contexts-2267ecce-105d-4bde-b47f-335b56635347.json b/change/@fluentui-react-shared-contexts-2267ecce-105d-4bde-b47f-335b56635347.json
@@ -0,0 +1,7 @@
+{
+  "type": "prerelease",
+  "comment": "make the ThemeContext TS API reflect runtime and remove redundant ThemeContextValue",
+  "packageName": "@fluentui/react-shared-contexts",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}
diff --git a/packages/react-provider/etc/react-provider.api.md b/packages/react-provider/etc/react-provider.api.md
@@ -12,7 +12,6 @@ import type { ProviderContextValue } from '@fluentui/react-shared-contexts';
 import * as React_2 from 'react';
 import type { Theme } from '@fluentui/react-theme';
 import type { ThemeClassNameContextValue } from '@fluentui/react-shared-contexts';
-import type { ThemeContextValue } from '@fluentui/react-shared-contexts';
 import type { TooltipContextType } from '@fluentui/react-shared-contexts';
 import { useFluent } from '@fluentui/react-shared-contexts';
 import { useTheme } from '@fluentui/react-shared-contexts';
@@ -30,12 +29,10 @@ export interface FluentProviderCommons {
 }
 
 // @public (undocumented)
-export interface FluentProviderContextValues {
+export interface FluentProviderContextValues extends Pick<FluentProviderState, 'theme'> {
     // (undocumented)
     provider: ProviderContextValue;
     // (undocumented)
-    theme: ThemeContextValue;
-    // (undocumented)
     themeClassName: ThemeClassNameContextValue;
     // (undocumented)
     tooltip: TooltipContextType;
@@ -55,7 +52,7 @@ export type FluentProviderSlots = {
 // @public (undocumented)
 export interface FluentProviderState extends ComponentState<FluentProviderSlots>, FluentProviderCommons {
     // (undocumented)
-    theme: Theme;
+    theme: Theme | Partial<Theme> | undefined;
     // (undocumented)
     themeClassName: string;
 }

diff --git a/packages/react-provider/src/components/FluentProvider/FluentProvider.test.tsx b/packages/react-provider/src/components/FluentProvider/FluentProvider.test.tsx
@@ -6,6 +6,13 @@ import { isConformant } from '../../common/isConformant';
 import { ProviderContext } from '@fluentui/react-shared-contexts';
 
 describe('FluentProvider', () => {
+  // eslint-disable-next-line @typescript-eslint/no-empty-function
+  const noop = () => {};
+
+  beforeEach(() => {
+    jest.spyOn(console, 'warn').mockImplementation(noop);
+  });
+
   isConformant({
     disabledTests: ['component-handles-classname'],
     Component: FluentProvider,
@@ -22,7 +29,9 @@ describe('FluentProvider', () => {
    * Note: see more visual regression tests for FluentProvider in /apps/vr-tests.
    */
   it('renders a default state', () => {
-    const component = renderer.create(<FluentProvider>Default FluentProvider</FluentProvider>);
+    const component = renderer.create(
+      <FluentProvider theme={{ colorBrandBackground2: '#fff' }}>Default FluentProvider</FluentProvider>,
+    );
     const tree = component.toJSON();
     expect(tree).toMatchSnapshot();
   });

diff --git a/packages/react-provider/src/components/FluentProvider/FluentProvider.types.ts b/packages/react-provider/src/components/FluentProvider/FluentProvider.types.ts
@@ -3,7 +3,6 @@ import type { PartialTheme, Theme } from '@fluentui/react-theme';
 import type {
   ProviderContextValue,
   TooltipContextType,
-  ThemeContextValue,
   ThemeClassNameContextValue,
 } from '@fluentui/react-shared-contexts';
 
@@ -26,13 +25,12 @@ export interface FluentProviderProps
 }
 
 export interface FluentProviderState extends ComponentState<FluentProviderSlots>, FluentProviderCommons {
-  theme: Theme;
+  theme: Theme | Partial<Theme> | undefined;
   themeClassName: string;
 }
 
-export interface FluentProviderContextValues {
+export interface FluentProviderContextValues extends Pick<FluentProviderState, 'theme'> {
   provider: ProviderContextValue;
-  theme: ThemeContextValue;
   themeClassName: ThemeClassNameContextValue;
   tooltip: TooltipContextType;
 }
diff --git a/packages/react-provider/src/components/FluentProvider/useFluentProvider.test.tsx b/packages/react-provider/src/components/FluentProvider/useFluentProvider.test.tsx
@@ -6,6 +6,26 @@ import { useFluentProvider } from './useFluentProvider';
 import type { PartialTheme } from '@fluentui/react-theme';
 
 describe('useFluentProvider', () => {
+  // eslint-disable-next-line @typescript-eslint/no-empty-function
+  const noop = () => {};
+  let logWarnSpy: jest.SpyInstance;
+
+  beforeEach(() => {
+    logWarnSpy = jest.spyOn(console, 'warn').mockImplementation(noop);
+  });
+
+  it(`should warn user if no theme was set in parent or child`, () => {
+    const Wrapper: React.FC = ({ children }) => <FluentProvider>{children}</FluentProvider>;
+
+    const { result } = renderHook(() => useFluentProvider({}, React.createRef()), {
+      wrapper: Wrapper,
+    });
+
+    expect(result.current.theme).toBe(undefined);
+    expect(logWarnSpy).toHaveBeenCalledTimes(2);
+    expect(logWarnSpy).toHaveBeenCalledWith(expect.stringContaining('FluentProvider: your "theme" is not defined !'));
+  });
+
   it('should merge themes', () => {
     const themeA: PartialTheme = {
       strokeWidthThick: '10px',

diff --git a/packages/react-provider/src/components/FluentProvider/useFluentProvider.ts b/packages/react-provider/src/components/FluentProvider/useFluentProvider.ts
@@ -24,9 +24,21 @@ export const useFluentProvider = (props: FluentProviderProps, ref: React.Ref<HTM
    * nesting providers with the same "dir" should not add additional attributes to DOM
    * see https://github.com/microsoft/fluentui/blob/0dc74a19f3aa5a058224c20505016fbdb84db172/packages/fluentui/react-northstar/src/utils/mergeProviderContexts.ts#L89-L93
    */
-  const { dir = parentContext.dir, targetDocument = parentContext.targetDocument, theme = {} } = props;
+  const { dir = parentContext.dir, targetDocument = parentContext.targetDocument, theme } = props;
   const mergedTheme = mergeThemes(parentTheme, theme);
 
+  React.useEffect(() => {
+    if (process.env.NODE_ENV !== 'production' && mergedTheme === undefined) {
+      // eslint-disable-next-line no-console
+      console.warn(`
+      FluentProvider: your "theme" is not defined !
+      =============================================
+      Make sure your root FluentProvider has set a theme or you're setting the theme in your child FluentProvider.
+      `);
+    }
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, []);
+
   return {
     dir,
     targetDocument,
@@ -45,7 +57,7 @@ export const useFluentProvider = (props: FluentProviderProps, ref: React.Ref<HTM
   };
 };
 
-function mergeThemes(a: Theme | undefined, b: Partial<Theme> | Theme | undefined): Theme {
+function mergeThemes(a: Theme | Partial<Theme> | undefined, b: typeof a): Theme | Partial<Theme> | undefined {
   // Merge impacts perf: we should like to avoid it if it's possible
   if (a && b) {
     return { ...a, ...b };
@@ -55,5 +67,5 @@ function mergeThemes(a: Theme | undefined, b: Partial<Theme> | Theme | undefined
     return a;
   }
 
-  return b as Theme;
+  return b;
 }
diff --git a/packages/react-provider/src/components/FluentProvider/useFluentProviderContextValues.test.ts b/packages/react-provider/src/components/FluentProvider/useFluentProviderContextValues.test.ts
@@ -5,6 +5,13 @@ import { useFluentProvider } from './useFluentProvider';
 import { useFluentProviderContextValues } from './useFluentProviderContextValues';
 
 describe('useFluentProviderContextValues', () => {
+  // eslint-disable-next-line @typescript-eslint/no-empty-function
+  const noop = () => {};
+
+  beforeEach(() => {
+    jest.spyOn(console, 'warn').mockImplementation(noop);
+  });
+
   it('should return a value for "provider"', () => {
     const { result } = renderHook(() => {
       const state = useFluentProvider({}, React.createRef());
@@ -24,17 +31,27 @@ describe('useFluentProviderContextValues', () => {
       return useFluentProviderContextValues(state);
     });
 
-    expect(result.current.tooltip).toBeDefined();
+    expect(result.current.tooltip).toEqual({});
   });
 
-  it('should return a value for "theme"', () => {
+  it('should return undefined if "theme" is not set', () => {
     const { result } = renderHook(() => {
       const state = useFluentProvider({}, React.createRef());
 
       return useFluentProviderContextValues(state);
     });
 
-    expect(result.current.theme).toBeDefined();
+    expect(result.current.theme).toBe(undefined);
+  });
+
+  it('should return a value for "theme"', () => {
+    const { result } = renderHook(() => {
+      const state = useFluentProvider({ theme: { colorBrandBackground: '#fff' } }, React.createRef());
+
+      return useFluentProviderContextValues(state);
+    });
+
+    expect(result.current.theme).toEqual({ colorBrandBackground: '#fff' });
   });
 
   it('should return a value for "themeClassname"', () => {
@@ -44,6 +61,6 @@ describe('useFluentProviderContextValues', () => {
       return useFluentProviderContextValues(state);
     });
 
-    expect(typeof result.current.themeClassName).toBe('string');
+    expect(result.current.themeClassName).toBe('foo');
   });
 });
diff --git a/packages/react-provider/src/components/FluentProvider/useThemeStyleTag.ts b/packages/react-provider/src/components/FluentProvider/useThemeStyleTag.ts
@@ -24,10 +24,12 @@ export const useThemeStyleTag = (options: Pick<FluentProviderState, 'theme' | 't
   }, [styleTagId, targetDocument]);
 
   const cssRule = React.useMemo(() => {
-    const cssVarsAsString = Object.keys(theme).reduce((cssVarRule, cssVar) => {
-      cssVarRule += `--${cssVar}: ${theme[cssVar as keyof typeof theme]}; `;
-      return cssVarRule;
-    }, '');
+    const cssVarsAsString = theme
+      ? (Object.keys(theme) as (keyof typeof theme)[]).reduce((cssVarRule, cssVar) => {
+          cssVarRule += `--${cssVar}: ${theme[cssVar]}; `;
+          return cssVarRule;
+        }, '')
+      : '';
 
     // result: .fluent-provider1 { --css-var: '#fff' }
     return `.${styleTagId} { ${cssVarsAsString} }`;
@@ -48,8 +50,7 @@ export const useThemeStyleTag = (options: Pick<FluentProviderState, 'theme' | 't
   React.useEffect(() => {
     return () => {
       if (styleTag) {
-        // IE11 safe node removal, otherwise use node.remove()
-        styleTag.parentElement?.removeChild(styleTag);
+        styleTag.remove();
       }
     };
   }, [styleTag]);

diff --git a/packages/react-shared-contexts/etc/react-shared-contexts.api.md b/packages/react-shared-contexts/etc/react-shared-contexts.api.md
@@ -33,11 +33,7 @@ export const ThemeClassNameContext: React_2.Context<string>;
 export type ThemeClassNameContextValue = string;
 
 // @public (undocumented)
-export const ThemeContext: React_2.Context<ThemeContextValue>;
-
-// @public (undocumented)
-export interface ThemeContextValue extends Theme {
-}
+export const ThemeContext: React_2.Context<Theme | Partial<Theme> | undefined>;
 
 // @public
 export const TooltipContext: React_2.Context<TooltipContextType>;
@@ -56,7 +52,7 @@ export function useFluent(): ProviderContextValue;
 export const useMenuContext: () => MinimalMenuProps;
 
 // @public (undocumented)
-export function useTheme(): ThemeContextValue;
+export function useTheme(): Theme | Partial<Theme> | undefined;
 
 // @public (undocumented)
 export function useThemeClassName(): ThemeClassNameContextValue;

diff --git a/packages/react-shared-contexts/src/ThemeContext/ThemeContext.ts b/packages/react-shared-contexts/src/ThemeContext/ThemeContext.ts
@@ -1,8 +1,8 @@
 import * as React from 'react';
-import type { ThemeContextValue } from './ThemeContext.types';
+import type { Theme } from '@fluentui/react-theme';
 
-export const ThemeContext = React.createContext((null as unknown) as ThemeContextValue);
+export const ThemeContext = React.createContext<Theme | Partial<Theme> | undefined>(undefined);
 
-export function useTheme(): ThemeContextValue {
+export function useTheme(): Theme | Partial<Theme> | undefined {
   return React.useContext(ThemeContext);
 }
diff --git a/packages/react-shared-contexts/src/ThemeContext/ThemeContext.types.ts b/packages/react-shared-contexts/src/ThemeContext/ThemeContext.types.ts
diff --git a/packages/react-shared-contexts/src/ThemeContext/index.ts b/packages/react-shared-contexts/src/ThemeContext/index.ts
@@ -1,2 +1 @@
 export * from './ThemeContext';
-export * from './ThemeContext.types';
Original file line number	Diff line number	Diff line change
		@@ -1,2 +1 @@
		export * from './ThemeContext';
		export * from './ThemeContext.types';