diff --git a/CHANGELOG.md b/CHANGELOG.md index 28abc36e7b..6865d68ec0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### BREAKING CHANGES - Restricted prop set in the `Checkbox`, `Icon`, `Label`, `Slider`, `Status`, `Text` @layershifter ([#2307](https://github.com/microsoft/fluent-ui-react/pull/2307)) +- Styles caching when no inline overrides are defined is enabled by default; use the `performance` prop on the `Provider` to opt out of this if needed @mnajdova ([#2309](https://github.com/microsoft/fluent-ui-react/pull/2309)) ### Fixes - Remove dependency on Lodash in TypeScript typings @layershifter ([#2323](https://github.com/microsoft/fluent-ui-react/pull/2323)) @@ -28,6 +29,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Features - Added sourcemaps to the dist output to simplify debugging @miroslavstastny ([#2329](https://github.com/microsoft/fluent-ui-react/pull/2329)) +### Performance +- Add styles caching when there aren't inline overrides defined @mnajdova ([#2309](https://github.com/microsoft/fluent-ui-react/pull/2309)) + ## [v0.44.0](https://github.com/microsoft/fluent-ui-react/tree/v0.44.0) (2020-02-05) [Compare changes](https://github.com/microsoft/fluent-ui-react/compare/v0.43.2..v0.44.0) diff --git a/packages/react-bindings/src/hooks/useStyles.ts b/packages/react-bindings/src/hooks/useStyles.ts index e82feac362..3765e22ba1 100644 --- a/packages/react-bindings/src/hooks/useStyles.ts +++ b/packages/react-bindings/src/hooks/useStyles.ts @@ -12,12 +12,12 @@ import { ThemeContext } from 'react-fela' import { ComponentDesignProp, ComponentSlotClasses, + PrimitiveProps, RendererRenderRule, StylesContextValue, } from '../styles/types' import getStyles from '../styles/getStyles' -type PrimitiveProps = Record type UseStylesOptions = { className?: string mapPropsToStyles?: () => StyleProps @@ -45,9 +45,12 @@ type InlineStyleProps = { const defaultContext: StylesContextValue<{ renderRule: RendererRenderRule }> = { disableAnimations: false, + performance: { + enableStylesCaching: true, + enableVariablesCaching: true, + }, renderer: { renderRule: () => '' }, theme: emptyTheme, - _internal_resolvedComponentVariables: {}, } const useStyles = ( @@ -57,6 +60,8 @@ const useStyles = ( const context: StylesContextValue<{ renderRule: RendererRenderRule }> = React.useContext(ThemeContext) || defaultContext + const { enableStylesCaching = true, enableVariablesCaching = true } = context.performance || {} + const { className = process.env.NODE_ENV === 'production' ? '' : 'no-classname-🙉', mapPropsToStyles = () => ({} as StyleProps), @@ -81,7 +86,10 @@ const useStyles = ( rtl, saveDebug: fluentUIDebug => (debug.current = { fluentUIDebug }), theme: context.theme, - _internal_resolvedComponentVariables: context._internal_resolvedComponentVariables, + performance: { + enableStylesCaching, + enableVariablesCaching, + }, }) return { classes, styles: resolvedStyles } diff --git a/packages/react-bindings/src/styles/getStyles.ts b/packages/react-bindings/src/styles/getStyles.ts index 514145dee6..3ab17b5190 100644 --- a/packages/react-bindings/src/styles/getStyles.ts +++ b/packages/react-bindings/src/styles/getStyles.ts @@ -1,39 +1,14 @@ import { - callable, - ComponentSlotStylesInput, - ComponentSlotStylesPrepared, ComponentSlotStylesResolved, - ComponentStyleFunctionParam, ComponentVariablesObject, - DebugData, - ICSSInJSStyle, isDebugEnabled, - mergeComponentStyles, - mergeComponentVariables, - PropsWithVarsAndStyles, - withDebugId, } from '@fluentui/styles' -import cx from 'classnames' + import * as _ from 'lodash' -import resolveStylesAndClasses from './resolveStylesAndClasses' -import { - ComponentDesignProp, - ComponentSlotClasses, - RendererParam, - RendererRenderRule, - StylesContextValue, -} from '../styles/types' - -type GetStylesOptions = StylesContextValue<{ - renderRule: RendererRenderRule -}> & { - className?: string - displayName: string - props: PropsWithVarsAndStyles & { design?: ComponentDesignProp } - rtl: boolean - saveDebug: (debug: DebugData | null) => void -} +import { ComponentSlotClasses, ResolveStylesOptions, StylesContextValue } from '../styles/types' +import resolveVariables from './resolveVariables' +import resolveStyles from './resolveStyles' export type GetStylesResult = { classes: ComponentSlotClasses @@ -42,86 +17,33 @@ export type GetStylesResult = { theme: StylesContextValue['theme'] } -const getStyles = (options: GetStylesOptions): GetStylesResult => { - const { - className, - disableAnimations, - displayName, - props, - renderer, - rtl, - saveDebug, - theme, - _internal_resolvedComponentVariables: resolvedComponentVariables, - } = options - - // Resolve variables for this component, cache the result in provider - if (!resolvedComponentVariables[displayName]) { - resolvedComponentVariables[displayName] = - callable(theme.componentVariables[displayName])(theme.siteVariables) || {} // component variables must not be undefined/null (see mergeComponentVariables contract) - } - - // Merge inline variables on top of cached variables - const resolvedVariables = props.variables - ? mergeComponentVariables( - resolvedComponentVariables[displayName], - withDebugId(props.variables, 'props.variables'), - )(theme.siteVariables) - : resolvedComponentVariables[displayName] - - // Resolve styles using resolved variables, merge results, allow props.styles to override - let mergedStyles: ComponentSlotStylesPrepared = theme.componentStyles[displayName] || { - root: () => ({}), - } - - const hasInlineOverrides = !_.isNil(props.design) || !_.isNil(props.styles) - - if (hasInlineOverrides) { - mergedStyles = mergeComponentStyles( - mergedStyles, - props.design && withDebugId({ root: props.design }, 'props.design'), - props.styles && - withDebugId({ root: props.styles } as ComponentSlotStylesInput, 'props.styles'), - ) - } - - const styleParam: ComponentStyleFunctionParam = { - displayName, - props, - variables: resolvedVariables, - theme, - rtl, - disableAnimations, - } - - // Fela plugins rely on `direction` param in `theme` prop instead of RTL - // Our API should be aligned with it - // Heads Up! Keep in sync with Design.tsx render logic - const direction = rtl ? 'rtl' : 'ltr' - const felaParam: RendererParam = { - theme: { direction }, - disableAnimations, - displayName, // does not affect styles, only used by useEnhancedRenderer in docs - } - - const { resolvedStyles, resolvedStylesDebug, classes } = resolveStylesAndClasses( - mergedStyles, - styleParam, - (style: ICSSInJSStyle) => renderer.renderRule(() => style, felaParam), +const getStyles = (options: ResolveStylesOptions): GetStylesResult => { + // + // To compute styles we are going through three stages: + // - resolve variables (siteVariables => componentVariables + props.variables) + // - resolve styles (with resolvedVariables & props.styles & props.design) + // - compute classes (with resolvedStyles) + // - conditionally add sources for evaluating debug information to component + + const resolvedVariables = resolveVariables( + options.displayName, + options.theme, + options.props.variables, + options.performance.enableVariablesCaching, ) - classes.root = cx(className, classes.root, props.className) + const { classes, resolvedStyles, resolvedStylesDebug } = resolveStyles(options, resolvedVariables) // conditionally add sources for evaluating debug information to component if (process.env.NODE_ENV !== 'production' && isDebugEnabled) { - saveDebug({ - componentName: displayName, + options.saveDebug({ + componentName: options.displayName, componentVariables: _.filter( resolvedVariables._debug, variables => !_.isEmpty(variables.resolved), ), componentStyles: resolvedStylesDebug, - siteVariables: _.filter(theme.siteVariables._debug, siteVars => { + siteVariables: _.filter(options.theme.siteVariables._debug, siteVars => { if (_.isEmpty(siteVars) || _.isEmpty(siteVars.resolved)) { return false } @@ -144,7 +66,7 @@ const getStyles = (options: GetStylesOptions): GetStylesResult => { classes, variables: resolvedVariables, styles: resolvedStyles, - theme, + theme: options.theme, } } diff --git a/packages/react-bindings/src/styles/resolveStyles.ts b/packages/react-bindings/src/styles/resolveStyles.ts new file mode 100644 index 0000000000..8d9dcda323 --- /dev/null +++ b/packages/react-bindings/src/styles/resolveStyles.ts @@ -0,0 +1,227 @@ +import cx from 'classnames' +import { + ComponentSlotStylesInput, + ComponentSlotStylesPrepared, + ComponentSlotStylesResolved, + ComponentStyleFunctionParam, + ComponentVariablesObject, + ICSSInJSStyle, + isDebugEnabled, + mergeComponentStyles, + ThemePrepared, + withDebugId, +} from '@fluentui/styles' +import { ComponentSlotClasses, RendererParam, ResolveStylesOptions } from '@fluentui/react-bindings' +import * as _ from 'lodash' + +export type ResolveStylesResult = { + resolvedStyles: ComponentSlotStylesResolved + resolvedStylesDebug: Record + classes: ComponentSlotClasses +} + +// this weak map is used as cache for the classes +const classesCache = new WeakMap>() + +// this weak map is used as cache for the styles +const stylesCache = new WeakMap>() + +/** + * Both resolvedStyles and classes are objects of getters with lazy evaluation + * + * Additionally if the cacheEnabled option is provided, than the resolved styles + * and classes are caching the results in WeakMaps. The key of the maps contains the following: + * - theme + * - displayName + * - slot name + * - styling props + * - rtl mode + * - disable animations mode + */ +const resolveStyles = ( + options: ResolveStylesOptions, + resolvedVariables: ComponentVariablesObject, + renderStylesInput?: (styles: ICSSInJSStyle) => string, +): ResolveStylesResult => { + const { + className: componentClassName, + theme, + displayName, + props, + rtl, + disableAnimations, + renderer, + performance = {}, + } = options || {} + + const { className, design, styles, variables, ...stylesProps } = props + const noInlineOverrides = !(design || styles || variables) + + const cacheEnabled = performance.enableStylesCaching && noInlineOverrides + + // Merge theme styles with inline overrides if any + let mergedStyles: ComponentSlotStylesPrepared = theme.componentStyles[displayName] || { + root: () => ({}), + } + const hasInlineStylesOverrides = !_.isNil(props.design) || !_.isNil(props.styles) + + if (hasInlineStylesOverrides) { + mergedStyles = mergeComponentStyles( + mergedStyles, + props.design && withDebugId({ root: props.design }, 'props.design'), + props.styles && + withDebugId({ root: props.styles } as ComponentSlotStylesInput, 'props.styles'), + ) + } + + const styleParam: ComponentStyleFunctionParam = { + displayName, + props, + variables: resolvedVariables, + theme, + rtl, + disableAnimations, + } + + // Fela plugins rely on `direction` param in `theme` prop instead of RTL + // Our API should be aligned with it + // Heads Up! Keep in sync with Design.tsx render logic + const direction = rtl ? 'rtl' : 'ltr' + const felaParam: RendererParam = { + theme: { direction }, + disableAnimations, + displayName, // does not affect styles, only used by useEnhancedRenderer in docs + } + + const renderStyles = + renderStylesInput || ((style: ICSSInJSStyle) => renderer.renderRule(() => style, felaParam)) + + const resolvedStyles: Record = {} + const resolvedStylesDebug: Record = {} + const classes: Record = {} + + if (cacheEnabled && theme) { + if (!stylesCache.has(theme)) { + stylesCache.set(theme, {}) + } + if (!classesCache.has(theme)) { + classesCache.set(theme, {}) + } + } + + const componentCacheKey = + cacheEnabled && displayName && stylesProps + ? `${displayName}:${JSON.stringify(stylesProps)}${styleParam.rtl}${ + styleParam.disableAnimations + }` + : '' + + Object.keys(mergedStyles).forEach(slotName => { + // resolve/render slot styles once and cache + const lazyEvaluationKey = `${slotName}__return` + const slotCacheKey = componentCacheKey + slotName + + Object.defineProperty(resolvedStyles, slotName, { + enumerable: false, + configurable: false, + set(val: ICSSInJSStyle) { + // Add to the cache if it's enabled + if (cacheEnabled && theme) { + stylesCache.set(theme, { + ...stylesCache.get(theme), + [slotCacheKey]: val, + }) + } + + resolvedStyles[lazyEvaluationKey] = val + }, + get(): ICSSInJSStyle { + // If caching enabled and entry exists, get from cache, avoid lazy evaluation + if (cacheEnabled && theme) { + const stylesThemeCache = stylesCache.get(theme) || {} + if (stylesThemeCache[slotCacheKey]) { + return stylesThemeCache[slotCacheKey] + } + } + + if (resolvedStyles[lazyEvaluationKey]) { + return resolvedStyles[lazyEvaluationKey] + } + + // resolve/render slot styles once and cache + resolvedStyles[lazyEvaluationKey] = mergedStyles[slotName](styleParam) + + if (cacheEnabled && theme) { + stylesCache.set(theme, { + ...stylesCache.get(theme), + [slotCacheKey]: resolvedStyles[lazyEvaluationKey], + }) + } + + if (process.env.NODE_ENV !== 'production' && isDebugEnabled) { + resolvedStylesDebug[slotName] = resolvedStyles[slotName]['_debug'] + delete resolvedStyles[slotName]['_debug'] + } + + return resolvedStyles[lazyEvaluationKey] + }, + }) + + Object.defineProperty(classes, slotName, { + enumerable: false, + configurable: false, + set(val: string) { + if (cacheEnabled && theme) { + classesCache.set(theme, { + ...classesCache.get(theme), + [slotCacheKey]: val, + }) + } + + classes[lazyEvaluationKey] = val + }, + get(): string { + if (cacheEnabled && theme) { + const classesThemeCache = classesCache.get(theme) || {} + if (classesThemeCache[slotCacheKey]) { + return slotName === 'root' + ? cx(componentClassName, classesThemeCache[slotCacheKey], className) + : classesThemeCache[slotCacheKey] + } + } + + if (classes[lazyEvaluationKey]) { + return slotName === 'root' + ? cx(componentClassName, classes[lazyEvaluationKey], className) + : classes[lazyEvaluationKey] + } + + // this resolves the getter magic + const styleObj = resolvedStyles[slotName] + + if (renderStyles && styleObj) { + classes[lazyEvaluationKey] = renderStyles(styleObj) + + if (cacheEnabled && theme) { + classesCache.set(theme, { + ...classesCache.get(theme), + [slotCacheKey]: classes[lazyEvaluationKey], + }) + } + } + + return slotName === 'root' + ? cx(componentClassName, classes[lazyEvaluationKey], className) + : classes[lazyEvaluationKey] + }, + }) + }) + + return { + resolvedStyles, + resolvedStylesDebug, + classes, + } +} + +export default resolveStyles diff --git a/packages/react-bindings/src/styles/resolveStylesAndClasses.tsx b/packages/react-bindings/src/styles/resolveStylesAndClasses.tsx deleted file mode 100644 index 38fa28c875..0000000000 --- a/packages/react-bindings/src/styles/resolveStylesAndClasses.tsx +++ /dev/null @@ -1,82 +0,0 @@ -import { - ComponentSlotStylesPrepared, - isDebugEnabled, - ICSSInJSStyle, - ComponentStyleFunctionParam, -} from '@fluentui/styles' -import { ComponentSlotClasses } from '../styles/types' - -// Both resolvedStyles and classes are objects of getters with lazy evaluation -const resolveStylesAndClasses = ( - mergedStyles: ComponentSlotStylesPrepared, - styleParam: ComponentStyleFunctionParam, - renderStyles: (styles: ICSSInJSStyle) => string, -): { - resolvedStyles: ICSSInJSStyle - resolvedStylesDebug: Record - classes: ComponentSlotClasses -} => { - const resolvedStyles: Record = {} - const resolvedStylesDebug: Record = {} - const classes: Record = {} - - Object.keys(mergedStyles).forEach(slotName => { - // resolve/render slot styles once and cache - const cacheKey = `${slotName}__return` - - Object.defineProperty(resolvedStyles, slotName, { - enumerable: false, - configurable: false, - set(val) { - resolvedStyles[cacheKey] = val - return true - }, - get() { - if (resolvedStyles[cacheKey]) { - return resolvedStyles[cacheKey] - } - - // resolve/render slot styles once and cache - resolvedStyles[cacheKey] = mergedStyles[slotName](styleParam) - - if (process.env.NODE_ENV !== 'production' && isDebugEnabled) { - resolvedStylesDebug[slotName] = resolvedStyles[slotName]['_debug'] - delete resolvedStyles[slotName]['_debug'] - } - - return resolvedStyles[cacheKey] - }, - }) - - Object.defineProperty(classes, slotName, { - enumerable: false, - configurable: false, - set(val) { - classes[cacheKey] = val - return true - }, - get() { - if (classes[cacheKey]) { - return classes[cacheKey] - } - - // this resolves the getter magic - const styleObj = resolvedStyles[slotName] - - if (renderStyles && styleObj) { - classes[cacheKey] = renderStyles(styleObj) - } - - return classes[cacheKey] - }, - }) - }) - - return { - resolvedStyles, - resolvedStylesDebug, - classes, - } -} - -export default resolveStylesAndClasses diff --git a/packages/react-bindings/src/styles/resolveVariables.ts b/packages/react-bindings/src/styles/resolveVariables.ts new file mode 100644 index 0000000000..0f3a8db47a --- /dev/null +++ b/packages/react-bindings/src/styles/resolveVariables.ts @@ -0,0 +1,54 @@ +import { + callable, + ComponentVariablesInput, + ComponentVariablesObject, + mergeComponentVariables, + ThemePrepared, + withDebugId, +} from '@fluentui/styles' + +const variablesCache = new WeakMap>() + +const resolveVariables = ( + displayName: string, + theme: ThemePrepared, + variables: ComponentVariablesInput | undefined, + enabledVariablesCaching: boolean | undefined, +): ComponentVariablesObject => { + // + // Simple caching model, works only if there is no `props.variables` + // Resolves variables for this component, cache the result in provider + // + + let componentThemeVariables = {} + + if (enabledVariablesCaching) { + if (!variablesCache.has(theme)) { + variablesCache.set(theme, {}) + } + + const variablesThemeCache = variablesCache.get(theme) || {} + + if (!variablesThemeCache[displayName]) { + variablesThemeCache[displayName] = + callable(theme.componentVariables[displayName])(theme.siteVariables) || {} + variablesCache.set(theme, variablesThemeCache) + } + + componentThemeVariables = variablesThemeCache[displayName] + } else { + componentThemeVariables = + callable(theme.componentVariables[displayName])(theme.siteVariables) || {} + } + + if (variables === undefined) { + return componentThemeVariables + } + + return mergeComponentVariables( + componentThemeVariables, + withDebugId(variables, 'props.variables'), + )(theme.siteVariables) +} + +export default resolveVariables diff --git a/packages/react-bindings/src/styles/types.ts b/packages/react-bindings/src/styles/types.ts index 4472bf7e9d..fa4fc1a169 100644 --- a/packages/react-bindings/src/styles/types.ts +++ b/packages/react-bindings/src/styles/types.ts @@ -1,4 +1,10 @@ -import { ICSSInJSStyle, ThemeInput, ThemePrepared } from '@fluentui/styles' +import { + DebugData, + ICSSInJSStyle, + PropsWithVarsAndStyles, + ThemeInput, + ThemePrepared, +} from '@fluentui/styles' import { IRenderer as FelaRenderer } from 'fela' // Notice: @@ -61,13 +67,30 @@ export type Renderer = Omit & { renderRule: RendererRenderRule } +export interface StylesContextPerformance { + enableStylesCaching?: boolean + enableVariablesCaching?: boolean +} + export type StylesContextInputValue = { disableAnimations?: boolean + performance?: StylesContextPerformance renderer?: R theme?: ThemeInput } export type StylesContextValue = Required> & { theme: ThemePrepared - _internal_resolvedComponentVariables: Record +} + +export type PrimitiveProps = Record + +export type ResolveStylesOptions = StylesContextValue<{ + renderRule: RendererRenderRule +}> & { + className?: string + displayName: string + props: PropsWithVarsAndStyles & { design?: ComponentDesignProp } + rtl: boolean + saveDebug: (debug: DebugData | null) => void } diff --git a/packages/react-bindings/test/hooks/useStyles-test.tsx b/packages/react-bindings/test/hooks/useStyles-test.tsx index d97892855b..6eba9f4953 100644 --- a/packages/react-bindings/test/hooks/useStyles-test.tsx +++ b/packages/react-bindings/test/hooks/useStyles-test.tsx @@ -39,7 +39,6 @@ const TestProvider: React.FC<{ theme: ThemeInput }> = props => { {children} diff --git a/packages/react-bindings/test/styles/resolveStyles-test.ts b/packages/react-bindings/test/styles/resolveStyles-test.ts new file mode 100644 index 0000000000..ae8216cef1 --- /dev/null +++ b/packages/react-bindings/test/styles/resolveStyles-test.ts @@ -0,0 +1,269 @@ +import * as _ from 'lodash' +import { + ComponentSlotStylesPrepared, + ComponentVariablesObject, + emptyTheme, + ICSSInJSStyle, +} from '@fluentui/styles' +import resolveStyles from '../../src/styles/resolveStyles' +import { ResolveStylesOptions } from '../../src/styles/types' + +const componentStyles: ComponentSlotStylesPrepared<{}, { color: string }> = { + root: ({ variables: v }): ICSSInJSStyle => ({ + color: v.color, + }), +} + +const resolvedVariables: ComponentVariablesObject = { + color: 'red', +} + +const resolveStylesOptions = (options?: { + displayName?: ResolveStylesOptions['displayName'] + performance?: ResolveStylesOptions['performance'] + props?: ResolveStylesOptions['props'] +}): ResolveStylesOptions => { + const { + displayName = 'Test', + performance = { + enableVariablesCaching: true, + enableStylesCaching: true, + }, + props = {}, + } = options || {} + + return { + theme: { + ...emptyTheme, + componentStyles: { + [displayName]: componentStyles, + }, + }, + displayName, + props, + rtl: false, + disableAnimations: false, + renderer: { + renderRule: () => '', + }, + performance, + saveDebug: () => {}, + } +} + +describe('resolveStyles', () => { + test('resolves styles', () => { + const { resolvedStyles } = resolveStyles(resolveStylesOptions(), resolvedVariables) + + expect(resolvedStyles.root).toMatchObject({ color: 'red' }) + }) + + test('caches resolved styles', () => { + spyOn(componentStyles, 'root').and.callThrough() + const { resolvedStyles } = resolveStyles(resolveStylesOptions(), resolvedVariables) + + expect(resolvedStyles.root).toMatchObject({ color: 'red' }) + expect(componentStyles.root).toHaveBeenCalledTimes(1) + expect(resolvedStyles.root).toMatchObject({ color: 'red' }) + expect(componentStyles.root).toHaveBeenCalledTimes(1) + }) + + test('does not render classes if not fetched', () => { + const renderStyles = jest.fn() + const { resolvedStyles } = resolveStyles( + resolveStylesOptions(), + resolvedVariables, + renderStyles, + ) + + expect(resolvedStyles.root).toMatchObject({ color: 'red' }) + expect(renderStyles).not.toBeCalled() + }) + + test('renders classes when slot classes getter is accessed', () => { + const renderStyles = jest.fn().mockReturnValue('a') + const { classes } = resolveStyles(resolveStylesOptions(), resolvedVariables, renderStyles) + + expect(classes['root']).toBeDefined() + expect(renderStyles).toHaveBeenCalledWith({ color: 'red' }) + }) + + test('caches rendered classes', () => { + const renderStyles = jest.fn().mockReturnValue('a') + const { classes } = resolveStyles(resolveStylesOptions(), resolvedVariables, renderStyles) + + expect(classes['root']).toBeDefined() + expect(renderStyles).toHaveBeenCalledWith({ color: 'red' }) + expect(classes['root']).toBeDefined() + expect(renderStyles).toHaveBeenCalledTimes(1) + }) + + test('caches resolved styles for no props', () => { + spyOn(componentStyles, 'root').and.callThrough() + const options = resolveStylesOptions() + const { resolvedStyles } = resolveStyles(options, resolvedVariables) + const { resolvedStyles: secondResolvedStyles } = resolveStyles(options, resolvedVariables) + + expect(resolvedStyles.root).toMatchObject({ color: 'red' }) + expect(componentStyles.root).toHaveBeenCalledTimes(1) + expect(secondResolvedStyles.root).toMatchObject({ color: 'red' }) + expect(componentStyles.root).toHaveBeenCalledTimes(1) + }) + + test('caches classes for no props', () => { + const renderStyles = jest.fn().mockReturnValue('a') + const options = resolveStylesOptions({ displayName: 'Test1' }) + const { classes } = resolveStyles(options, resolvedVariables, renderStyles) + const { classes: secondClasses } = resolveStyles(options, resolvedVariables, renderStyles) + + expect(classes['root']).toBeDefined() + expect(renderStyles).toHaveBeenCalledWith({ color: 'red' }) + expect(secondClasses['root']).toBeDefined() + expect(renderStyles).toHaveBeenCalledTimes(1) + }) + + test('caches resolved styles for the same props', () => { + spyOn(componentStyles, 'root').and.callThrough() + const options = resolveStylesOptions({ + displayName: 'Test2', + props: { primary: true }, + }) + const { resolvedStyles } = resolveStyles(options, resolvedVariables) + const { resolvedStyles: secondResolvedStyles } = resolveStyles(options, resolvedVariables) + + expect(resolvedStyles.root).toMatchObject({ color: 'red' }) + expect(componentStyles.root).toHaveBeenCalledTimes(1) + expect(secondResolvedStyles.root).toMatchObject({ color: 'red' }) + expect(componentStyles.root).toHaveBeenCalledTimes(1) + }) + + test('caches classes for the same props', () => { + const renderStyles = jest.fn().mockReturnValue('a') + const options = resolveStylesOptions({ + displayName: 'Test3', + props: { primary: true }, + }) + const { classes } = resolveStyles(options, resolvedVariables, renderStyles) + const { classes: secondClasses } = resolveStyles(options, resolvedVariables, renderStyles) + + expect(classes['root']).toBeDefined() + expect(renderStyles).toHaveBeenCalledWith({ color: 'red' }) + expect(secondClasses['root']).toBeDefined() + expect(renderStyles).toHaveBeenCalledTimes(1) + }) + + test('considers props when caching resolved styles', () => { + spyOn(componentStyles, 'root').and.callThrough() + const options = resolveStylesOptions({ + displayName: 'Test4', + props: { primary: true }, + }) + const { resolvedStyles } = resolveStyles(options, resolvedVariables) + + options.props = { primary: false } + const { resolvedStyles: secondResolvedStyles } = resolveStyles(options, resolvedVariables) + + expect(resolvedStyles.root).toMatchObject({ color: 'red' }) + expect(componentStyles.root).toHaveBeenCalledTimes(1) + expect(secondResolvedStyles.root).toMatchObject({ color: 'red' }) + expect(componentStyles.root).toHaveBeenCalledTimes(2) + }) + + test('considers props when caching classes', () => { + const renderStyles = jest.fn().mockReturnValue('a') + const options = resolveStylesOptions({ + displayName: 'Test5', + props: { primary: true }, + }) + const { classes } = resolveStyles(options, resolvedVariables, renderStyles) + + options.props = { primary: false } + const { classes: secondClasses } = resolveStyles(options, resolvedVariables, renderStyles) + + expect(classes['root']).toBeDefined() + expect(renderStyles).toHaveBeenCalledWith({ color: 'red' }) + expect(secondClasses['root']).toBeDefined() + expect(renderStyles).toHaveBeenCalledTimes(2) + }) + + test('does not cache styles if caching is disabled', () => { + spyOn(componentStyles, 'root').and.callThrough() + const options = resolveStylesOptions({ performance: { enableStylesCaching: false } }) + const { resolvedStyles } = resolveStyles(options, resolvedVariables) + const { resolvedStyles: secondResolvedStyles } = resolveStyles(options, resolvedVariables) + + expect(resolvedStyles.root).toMatchObject({ color: 'red' }) + expect(secondResolvedStyles.root).toMatchObject({ color: 'red' }) + expect(componentStyles.root).toHaveBeenCalledTimes(2) + }) + + test('does not cache classes if caching is disabled', () => { + const renderStyles = jest.fn().mockReturnValue('a') + const options = resolveStylesOptions({ performance: { enableStylesCaching: false } }) + const { classes } = resolveStyles(options, resolvedVariables, renderStyles) + const { classes: secondClasses } = resolveStyles(options, resolvedVariables, renderStyles) + + expect(classes['root']).toBeDefined() + expect(renderStyles).toHaveBeenCalledWith({ color: 'red' }) + expect(secondClasses['root']).toBeDefined() + expect(renderStyles).toHaveBeenCalledTimes(2) + }) + + test('does not cache styles if there are inline overrides', () => { + spyOn(componentStyles, 'root').and.callThrough() + const propsInlineOverrides: ResolveStylesOptions['props'][] = [ + { styles: { fontSize: '10px' } }, + { design: { left: '10px' } }, + { variables: { backgroundColor: 'yellow' } }, + ] + + const propsInlineOverridesResolvedStyles: ICSSInJSStyle[] = [ + { color: 'red', fontSize: '10px' }, + { color: 'red', left: '10px' }, + { color: 'red' }, + ] + + const propsInlineOverridesSize = propsInlineOverrides.length + + _.forEach(propsInlineOverrides, (props, idx) => { + const options = resolveStylesOptions({ + props, + performance: { enableStylesCaching: false }, + }) + + const { resolvedStyles } = resolveStyles(options, resolvedVariables) + const { resolvedStyles: secondResolvedStyles } = resolveStyles(options, resolvedVariables) + + expect(resolvedStyles.root).toMatchObject(propsInlineOverridesResolvedStyles[idx]) + expect(secondResolvedStyles.root).toMatchObject(propsInlineOverridesResolvedStyles[idx]) + resolveStyles(options, resolvedVariables) + }) + + expect(componentStyles.root).toHaveBeenCalledTimes(propsInlineOverridesSize * 2) + }) + + test('does not cache classes if there are inline overrides', () => { + const renderStyles = jest.fn().mockReturnValue('a') + const propsInlineOverrides: ResolveStylesOptions['props'][] = [ + { styles: { fontSize: '10px' } }, + { design: { left: '10px' } }, + { variables: { backgroundColor: 'yellow' } }, + ] + + const propsInlineOverridesSize = propsInlineOverrides.length + + _.forEach(propsInlineOverrides, props => { + const options = resolveStylesOptions({ + props, + performance: { enableStylesCaching: false }, + }) + const { classes } = resolveStyles(options, resolvedVariables, renderStyles) + const { classes: secondClasses } = resolveStyles(options, resolvedVariables, renderStyles) + + expect(classes['root']).toBeDefined() + expect(secondClasses['root']).toBeDefined() + }) + + expect(renderStyles).toHaveBeenCalledTimes(propsInlineOverridesSize * 2) + }) +}) diff --git a/packages/react-bindings/test/styles/resolveStylesAndClasses-test.ts b/packages/react-bindings/test/styles/resolveStylesAndClasses-test.ts deleted file mode 100644 index 71ea32fdbe..0000000000 --- a/packages/react-bindings/test/styles/resolveStylesAndClasses-test.ts +++ /dev/null @@ -1,68 +0,0 @@ -import { - ComponentSlotStylesPrepared, - ComponentStyleFunctionParam, - emptyTheme, - ICSSInJSStyle, -} from '@fluentui/styles' -import resolveStylesAndClasses from '../../src/styles/resolveStylesAndClasses' - -const styleParam: ComponentStyleFunctionParam = { - disableAnimations: false, - displayName: 'Test', - props: {}, - rtl: false, - theme: emptyTheme, - variables: { - color: 'red', - }, -} - -const componentStyles: ComponentSlotStylesPrepared<{}, { color: string }> = { - root: ({ variables: v }): ICSSInJSStyle => ({ - color: v.color, - }), -} - -describe('resolveStylesAndClasses', () => { - test('resolves styles', () => { - const { resolvedStyles } = resolveStylesAndClasses(componentStyles, styleParam, () => '') - - expect(resolvedStyles.root).toMatchObject({ color: 'red' }) - }) - - test('caches resolved styles', () => { - spyOn(componentStyles, 'root').and.callThrough() - const { resolvedStyles } = resolveStylesAndClasses(componentStyles, styleParam, () => '') - - expect(resolvedStyles.root).toMatchObject({ color: 'red' }) - expect(componentStyles.root).toHaveBeenCalledTimes(1) - expect(resolvedStyles.root).toMatchObject({ color: 'red' }) - expect(componentStyles.root).toHaveBeenCalledTimes(1) - }) - - test('does not render classes if not fetched', () => { - const renderStyles = jest.fn() - const { resolvedStyles } = resolveStylesAndClasses(componentStyles, styleParam, renderStyles) - - expect(resolvedStyles.root).toMatchObject({ color: 'red' }) - expect(renderStyles).not.toBeCalled() - }) - - test('renders classes when slot classes getter is accessed', () => { - const renderStyles = jest.fn().mockReturnValue('a') - const { classes } = resolveStylesAndClasses(componentStyles, styleParam, renderStyles) - - expect(classes.root).toBeDefined() - expect(renderStyles).toHaveBeenCalledWith({ color: 'red' }) - }) - - test('caches rendered classes', () => { - const renderStyles = jest.fn().mockReturnValue('a') - const { classes } = resolveStylesAndClasses(componentStyles, styleParam, renderStyles) - - expect(classes.root).toBeDefined() - expect(renderStyles).toHaveBeenCalledWith({ color: 'red' }) - expect(classes.root).toBeDefined() - expect(renderStyles).toHaveBeenCalledTimes(1) - }) -}) diff --git a/packages/react-bindings/test/styles/resolveVariables-test.ts b/packages/react-bindings/test/styles/resolveVariables-test.ts new file mode 100644 index 0000000000..433d4bf28d --- /dev/null +++ b/packages/react-bindings/test/styles/resolveVariables-test.ts @@ -0,0 +1,86 @@ +import resolveVariables from '../../src/styles/resolveVariables' +import { ComponentVariablesPrepared, emptyTheme, ThemePrepared } from '@fluentui/styles' + +const siteVariables = { + ...emptyTheme.siteVariables, + brand: 'blue', +} +const componentVariables: ComponentVariablesPrepared = ( + siteVariables = emptyTheme.siteVariables, +) => ({ + backgroundColor: siteVariables.brand, +}) + +const createTheme: (displayName?: string) => ThemePrepared = displayName => ({ + ...emptyTheme, + siteVariables, + componentVariables: { + [displayName || 'Test']: componentVariables, + }, +}) + +describe('resolveVariables', () => { + test('resolved variables', () => { + const variables = resolveVariables('Test', createTheme('Test'), {}, false) + expect(variables).toMatchObject({ backgroundColor: 'blue' }) + }) + + test('merges theme with input variables', () => { + const propsVariables = () => ({ + color: 'red', + }) + const variables = resolveVariables('Test', createTheme('Test'), propsVariables, false) + expect(variables).toMatchObject({ backgroundColor: 'blue', color: 'red' }) + }) + + test("allows input variabes to override theme's", () => { + const propsVariables = { + backgroundColor: 'green', + } + const variables = resolveVariables('Test', createTheme('Test'), propsVariables, false) + expect(variables).toMatchObject({ backgroundColor: 'green' }) + }) + + test('caches resolved variables', () => { + const componentVariablesMock = jest.fn().mockReturnValue({ backgroundColor: 'blue' }) + const theme = createTheme() + theme.componentVariables['Test'] = componentVariablesMock + + const variables = resolveVariables('Test', theme, {}, true) + const secondVariables = resolveVariables('Test', theme, {}, true) + + expect(variables).toMatchObject(secondVariables) + expect(componentVariablesMock).toHaveBeenCalledTimes(1) + }) + + test('considers displayName while caching resolved variables', () => { + const componentVariablesMock1 = jest.fn().mockReturnValue({ backgroundColor: 'blue' }) + const componentVariablesMock2 = jest.fn().mockReturnValue({ color: 'red' }) + const theme = createTheme() + theme.componentVariables['Test1'] = componentVariablesMock1 + theme.componentVariables['Test2'] = componentVariablesMock2 + + const variables = resolveVariables('Test1', theme, {}, true) + const secondVariables = resolveVariables('Test2', theme, {}, true) + + expect(variables).toMatchObject({ backgroundColor: 'blue' }) + expect(secondVariables).toMatchObject({ color: 'red' }) + expect(componentVariablesMock1).toHaveBeenCalledTimes(1) + expect(componentVariablesMock2).toHaveBeenCalledTimes(1) + }) + + test('considers theme while caching resolved variables', () => { + const componentVariablesMock = jest.fn().mockReturnValue({ backgroundColor: 'blue' }) + const theme = createTheme('Test') + const secondTheme = createTheme('Test') + theme.componentVariables['Test'] = componentVariablesMock + secondTheme.componentVariables['Test'] = componentVariablesMock + + const variables = resolveVariables('Test', theme, {}, true) + const secondVariables = resolveVariables('Test', secondTheme, {}, true) + + expect(variables).toMatchObject({ backgroundColor: 'blue' }) + expect(secondVariables).toMatchObject({ backgroundColor: 'blue' }) + expect(componentVariablesMock).toHaveBeenCalledTimes(2) + }) +}) diff --git a/packages/react/src/components/Provider/Provider.tsx b/packages/react/src/components/Provider/Provider.tsx index 9a93ffb901..de02656fb5 100644 --- a/packages/react/src/components/Provider/Provider.tsx +++ b/packages/react/src/components/Provider/Provider.tsx @@ -3,6 +3,7 @@ import * as _ from 'lodash' import { getUnhandledProps, Renderer, + StylesContextPerformance, Telemetry, useIsomorphicLayoutEffect, } from '@fluentui/react-bindings' @@ -37,6 +38,7 @@ export interface ProviderProps extends ChildrenComponentProps { renderer?: Renderer rtl?: boolean disableAnimations?: boolean + performance?: StylesContextPerformance overwrite?: boolean target?: Document theme?: ThemeInput @@ -119,6 +121,7 @@ const Provider: React.FC> & { theme: props.theme, rtl: props.rtl, disableAnimations: props.disableAnimations, + performance: props.performance, renderer: props.renderer, target: props.target, telemetry, @@ -203,6 +206,12 @@ Provider.propTypes = { renderer: PropTypes.object as PropTypes.Validator, rtl: PropTypes.bool, disableAnimations: PropTypes.bool, + // Heads Up! + // Keep in sync with packages/react-bindings/src/styles/types.ts + performance: PropTypes.shape({ + enableStylesCaching: PropTypes.bool, + enableVariablesCaching: PropTypes.bool, + }), children: PropTypes.node.isRequired, overwrite: PropTypes.bool, target: PropTypes.object as PropTypes.Validator, diff --git a/packages/react/src/types.ts b/packages/react/src/types.ts index b86139f761..c979941e51 100644 --- a/packages/react/src/types.ts +++ b/packages/react/src/types.ts @@ -1,4 +1,9 @@ -import { StylesContextInputValue, StylesContextValue, Telemetry } from '@fluentui/react-bindings' +import { + StylesContextInputValue, + StylesContextValue, + StylesContextPerformance, + Telemetry, +} from '@fluentui/react-bindings' import * as React from 'react' import { ShorthandFactory } from './utils/factories' @@ -174,6 +179,7 @@ export interface ProviderContextInput extends StylesContextInputValue { rtl?: boolean target?: Document telemetry?: Telemetry + performance?: StylesContextPerformance } export interface ProviderContextPrepared extends StylesContextValue { diff --git a/packages/react/src/utils/mergeProviderContexts.ts b/packages/react/src/utils/mergeProviderContexts.ts index 1bd211f8cb..f3a630d2ba 100644 --- a/packages/react/src/utils/mergeProviderContexts.ts +++ b/packages/react/src/utils/mergeProviderContexts.ts @@ -33,6 +33,10 @@ export const mergeRenderers = (current: Renderer, next?: Renderer, target?: Docu return createdRenderer } +export const mergePerformanceOptions = (target, ...sources) => { + return Object.assign(target, ...sources) +} + export const mergeBooleanValues = (target, ...sources) => { return sources.reduce((acc, next) => { return typeof next === 'boolean' ? next : acc @@ -57,8 +61,11 @@ const mergeProviderContexts = ( rtl: false, disableAnimations: false, target: isBrowser() ? document : undefined, // eslint-disable-line no-undef + performance: { + enableStylesCaching: true, + enableVariablesCaching: true, + }, telemetry: undefined, - _internal_resolvedComponentVariables: {}, renderer: undefined, } @@ -87,6 +94,8 @@ const mergeProviderContexts = ( acc.disableAnimations = mergedDisableAnimations } + acc.performance = mergePerformanceOptions(acc.performance, next.performance || {}) + acc.telemetry = next.telemetry || acc.telemetry return acc diff --git a/packages/react/src/utils/renderComponent.tsx b/packages/react/src/utils/renderComponent.tsx index 86aea0ab14..72198378d7 100644 --- a/packages/react/src/utils/renderComponent.tsx +++ b/packages/react/src/utils/renderComponent.tsx @@ -68,6 +68,7 @@ const renderComponent =

( const { setStart, setEnd } = useTelemetry(displayName, context.telemetry) const rtl = context.rtl || false + const enableVariablesCaching = context.performance?.enableVariablesCaching setStart() @@ -91,7 +92,11 @@ const renderComponent =

( rtl, saveDebug, theme: context.theme || emptyTheme, - _internal_resolvedComponentVariables: context._internal_resolvedComponentVariables || {}, + performance: { + enableVariablesCaching: + typeof enableVariablesCaching === 'boolean' ? enableVariablesCaching : true, + enableStylesCaching: false, // we cannot enable caching for class components + }, }) const resolvedConfig: RenderResultConfig

= { diff --git a/packages/react/test/utils/withProvider.tsx b/packages/react/test/utils/withProvider.tsx index 03ce491b36..30a3ffc919 100644 --- a/packages/react/test/utils/withProvider.tsx +++ b/packages/react/test/utils/withProvider.tsx @@ -10,11 +10,11 @@ export const EmptyThemeProvider: React.FunctionComponent = ({ children }) => { const theme: ProviderContextPrepared = { renderer: felaRenderer, target: document, - _internal_resolvedComponentVariables: {}, disableAnimations: false, rtl: false, theme: emptyTheme, telemetry: undefined, + performance: {}, } return {children}