Skip to content
This repository was archived by the owner on Mar 4, 2020. It is now read-only.

[WIP] feat(create-component): add create component #2200

Closed
wants to merge 14 commits into from
Closed
9 changes: 9 additions & 0 deletions docs/src/components/Sidebar/Sidebar.tsx
Original file line number Diff line number Diff line change
@@ -393,6 +393,15 @@ class Sidebar extends React.Component<any, any> {
},
public: true,
},
{
key: 'prototype-create-component',
title: {
content: 'Create component',
as: NavLink,
to: 'prototype-create-component',
},
public: false,
},
]

const componentTreeSection = {
122 changes: 122 additions & 0 deletions docs/src/prototypes/createComponent/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
import * as React from 'react'
import { mergeCss } from '@uifabric/merge-styles'
import {
Provider,
FluentTheme,
PlannerFluentTheme,
FluentButton,
FluentMenu,
FluentMenuItem,
} from '@fluentui/react-theming'

const oddRedBorder = mergeCss({ border: '10px solid red' })
const example = mergeCss({ margin: 20 })

const MenuItemText = (props: any) => {
return <span {...props}>{props.children}</span>
}

// This is a bad API... :(
const items = [
{
slots: { text: MenuItemText, menu: FluentMenu },
slotProps: {
text: { id: 'blabla', children: 'Bla' },
menu: {
slotProps: {
items: [
{
slots: { text: MenuItemText },
slotProps: { text: { id: 'blabla', children: 'Boo' } },
},
{
slots: { text: MenuItemText },
slotProps: { text: { id: 'blabla', children: 'Coo' } },
},
],
},
},
},
rounded: true,
},
{ slots: { text: MenuItemText }, slotProps: { text: { id: 'blabla', children: 'Foo' } } },
]

// Much better in my opinion
// const items = [
// { slots: { text: MenuItemText }, text: { id: 'blabla', children: 'Bla' } },
// { slots: { text: MenuItemText }, text: { id: 'blabla', children: 'Foo' } }
// ];
Copy link
Member

Choose a reason for hiding this comment

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

Yep, you should update this PR based on your proposal here.


const Icon: React.FunctionComponent<any> = props => <span {...props}>@</span>
const ButtonThemedExample: React.FunctionComponent<{}> = props => {
const onClick = React.useCallback(() => console.log('clicked button'), [])
const variants = () => {
return (
<>
<div className={example}>
<FluentButton tiny>tiny</FluentButton>
</div>
<div className={example}>
<FluentButton large>large</FluentButton>
</div>
<div className={example}>
<FluentButton size="s">small</FluentButton>
<FluentButton size="m">medium</FluentButton>
<FluentButton size="l">large</FluentButton>
</div>

<div className={example}>
<FluentButton shadowed>shadowed</FluentButton>
</div>
<div className={example}>
<FluentButton
bigIcon={true}
slots={{ icon: Icon, primaryText: () => <span>BigIcon</span> }}
/>
</div>
<div className={example}>
<FluentButton id="sdasdas" shadowed tiny>
shadowed & tiny
</FluentButton>
</div>
<div className={example}>
<FluentButton onClick={onClick} shadowed tiny bigIcon>
Shadowed tiny bigIcon
</FluentButton>
</div>
<div className={example}>
<FluentButton onClick={onClick} beautiful>
Beautiful
</FluentButton>
</div>

<div className={example}>
<FluentButton onClick={onClick} className={oddRedBorder}>
Fluent Button with an odd red border
</FluentButton>
</div>
</>
)
}
return (
<div>
<h1>Fluent Theme</h1>
<Provider theme={FluentTheme}>{variants()}</Provider>

<h1>Planner Fluent Theme</h1>
<Provider theme={PlannerFluentTheme}>{variants()}</Provider>

<h1>Menu</h1>
<Provider theme={PlannerFluentTheme}>
<FluentMenu rounded slotProps={{ items }} />
<FluentMenuItem
slots={{ menu: FluentMenu }}
slotProps={{ menu: { slotProps: { items } } }}
/>
</Provider>
</div>
)
}

export default ButtonThemedExample
2 changes: 2 additions & 0 deletions docs/src/routes.tsx
Original file line number Diff line number Diff line change
@@ -49,6 +49,7 @@ import CustomScrollbarPrototype from './prototypes/customScrollbar'
import EditorToolbarPrototype from './prototypes/EditorToolbar'
import HexagonalAvatarPrototype from './prototypes/hexagonalAvatar'
import TablePrototype from './prototypes/table'
import CreateComponentPrototype from './prototypes/createComponent'

const Routes = () => (
<BrowserRouter basename={__BASENAME__}>
@@ -89,6 +90,7 @@ const Routes = () => (
/>
<Route exact path="/virtualized-tree" component={VirtualizedTreePrototype} />
<Route exact path="/prototype-copy-to-clipboard" component={CopyToClipboardPrototype} />
<Route exact path="/prototype-create-component" component={CreateComponentPrototype} />
<Route exact path="/faq" component={FAQ} />
<Route exact path="/accessibility" component={Accessibility} />
<Route exact path="/accessibility-behaviors" component={AccessibilityBehaviors} />
38 changes: 38 additions & 0 deletions packages/react-theming/src/components/Button/BaseButton.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import * as React from 'react';

/**
* TODO:
* 1) do we really need slots prop?
Copy link
Member

Choose a reason for hiding this comment

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

Do you mean keeping this in context only opposed to also having a slot?

<Provider theme={{
    components: {
      Button: {
        slots: { icon: MyIcon },
      }
    }
  }}
/>

Ignore the fact that I put this in theme.components. I just mean to show "top-level config via context" vs a prop.

vs

<Button slots={{ icon: MyIcon }} />

This would allow easy inline overrides of the slots, but is it necessary?

*/
interface IBaseButtonProps extends React.AllHTMLAttributes<any> {
slots?: any;
slotProps?: any;
}

export const ButtonText: React.FunctionComponent<any> = props => <span {...props}>my button</span>;

export const BaseButton: React.FunctionComponent<IBaseButtonProps> = props => {
const { slots, children, slotProps, ...rest } = props;
const {
root: Root = 'button',
icon: Icon,
primaryText: PrimaryText,
secondaryText: SecondaryText,
} = slots || {};
const { root = {}, icon = {}, primaryText = {}, secondaryText = {} } = slotProps || {};

const rootClassName = `${root.className || ''}${` ${rest.className}` || ''}`;
const content = children || (
<>
{Icon && <Icon {...icon} />}
{PrimaryText && <PrimaryText {...primaryText} />}
{SecondaryText && <SecondaryText {...secondaryText} />}
</>
);

return (
<Root {...root} {...rest} className={rootClassName}>
{content}
</Root>
);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import { BaseButton } from './BaseButton';
import { createComponent } from '../../create-component/createComponent';

export const FluentButton = createComponent('FluentButton', BaseButton);
22 changes: 22 additions & 0 deletions packages/react-theming/src/components/Menu/BaseMenu.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import * as React from 'react';
import { BaseMenuItem } from './BaseMenuItem';

interface IMenuProps {
className?: string;
slots?: any;
slotProps?: any;
}

export const BaseMenu: React.FunctionComponent<IMenuProps> = props => {
const { slotProps = {}, slots = {}, ...rest } = props;
const { item: MenuItem = BaseMenuItem, root: Root = 'div' } = slots;
const { root: rootProps = {}, items = [] } = slotProps;
const rootClassName = `${rootProps.className || ''}${` ${rest && rest.className}` || ''}`;
return (
<Root {...rootProps} {...rest} className={rootClassName}>
{items.map((item: any) => (
<MenuItem key={item.id} {...item} />
))}
</Root>
);
};
32 changes: 32 additions & 0 deletions packages/react-theming/src/components/Menu/BaseMenuItem.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import * as React from 'react';

interface IMenuItemProps {
className?: string;
slots?: any;
slotProps?: any;
}

export const BaseMenuItem: React.FunctionComponent<IMenuItemProps> = props => {
const { children, slots = {}, slotProps = {}, ...rest } = props;
const { root: Root = 'div', text: Text, icon: Icon, menu: Menu } = slots;
const {
root: rootProps = {},
text: textProps = {},
icon: iconProps = {},
menu: menuProps = {},
} = slotProps;
const rootClassName = `${rootProps.className || ''}${` ${rest && rest.className}` || ''}`;
const content = children || (
<>
{Icon && <Icon {...iconProps} />}
{Text && <Text {...textProps} />}
{Menu && <Menu {...menuProps} />}
</>
);

return (
<Root {...rootProps} {...rest} className={rootClassName}>
{content}
</Root>
);
};
9 changes: 9 additions & 0 deletions packages/react-theming/src/components/Menu/FluentMenu.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { BaseMenu } from './BaseMenu';
import { FluentMenuItem } from './FluentMenuItem';
import { createComponent } from '../../create-component/createComponent';

export const FluentMenu = createComponent('FluentMenu', BaseMenu, {
slots: {
item: FluentMenuItem,
},
});
13 changes: 13 additions & 0 deletions packages/react-theming/src/components/Menu/FluentMenuItem.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { BaseMenuItem } from './BaseMenuItem';
import { createComponent } from '../../create-component/createComponent';
// import { FluentMenu } from './'

export const FluentMenuItem = createComponent(
'FluentMenuItem',
BaseMenuItem,
// {
// slots: {
// menu: FluentMenu,
// }
// }
);
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import * as React from 'react';
// import { IBaseThemeShape } from './ThemeShape';

/*
interface IProviderProps<T extends IBaseThemeShape> {
theme: T;
}
*/

export const ProviderContext = React.createContext(null);

export const Provider: React.FunctionComponent<any> = props => {
return <ProviderContext.Provider value={props.theme}>{props.children}</ProviderContext.Provider>;
};
49 changes: 49 additions & 0 deletions packages/react-theming/src/create-component/ClassCache.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import { ClassCache, VariantBasedCacheKeyStrategy } from './ClassCache';

describe('ClassCache', () => {
it('allows access via theme and string', () => {
const c = new ClassCache();
const val = {};
const theme = {};
c.set(theme, 'foo-bar-baz', val);
expect(c.get(theme, 'foo-bar-baz')).toBe(val);
});

it('allows access via theme and multiple strings', () => {
const c = new ClassCache();
const val = {};
const theme = {};
c.set(theme, 'foo-bar-baz', val);
c.set(theme, 'foo-bar', {});
expect(c.get(theme, 'foo-bar-baz')).toBe(val);
});

it('returns null if entry not found', () => {
const c = new ClassCache();
expect(c.get({}, '')).toBeNull();
});

describe('getOrSet', () => {
it('allows for passing in of a default value', () => {
const c = new ClassCache();
const cacheEntry = {};
const theme = {};
const key = '';
const fetchedEntry: any = c.getOrSet(theme, key, cacheEntry);
expect(fetchedEntry).toBe(cacheEntry);
expect(c.get(theme, key)).toBe(cacheEntry);
});
});

describe('with automative cache key computation', () => {
it('handles cache key computation', () => {
const c = new ClassCache();
const val = {};
const theme = {};
c.set(theme, new VariantBasedCacheKeyStrategy(['a', 'b', 'c'], {}).toString(), val);
expect(c.get(theme, new VariantBasedCacheKeyStrategy(['a', 'b', 'c'], {}).toString())).toBe(
val,
);
});
});
});
47 changes: 47 additions & 0 deletions packages/react-theming/src/create-component/ClassCache.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
export class ClassCache {
private cache = new WeakMap();

public get(theme: {}, arg1: string): any {
const obj = this.cache.get(theme);
if (!obj) {
return null;
}
return obj[arg1] || null;
}

public set(theme: {}, arg1: string, val: {}) {
let themeEntry;
if (this.cache.get(theme)) {
themeEntry = this.cache.get(theme);
} else {
themeEntry = {};
this.cache.set(theme, themeEntry);
}
themeEntry[arg1] = val;
}

public getOrSet(theme: {}, key: string, cacheEntry: any): any {
const existing = this.get(theme, key);
if (existing !== undefined && existing !== null) {
return existing;
}
this.set(theme, key, cacheEntry);
return cacheEntry;
}
}

export class VariantBasedCacheKeyStrategy {
private computed: string;

constructor(private variants: string[] = [], private props: any = {}) {}

public toString() {
if (this.computed) {
return this.computed;
}
const computedRaw: any = {};
this.variants.slice().forEach(v => (computedRaw[v] = this.props[v]));
this.computed = JSON.stringify(computedRaw);
return this.computed;
}
}
100 changes: 100 additions & 0 deletions packages/react-theming/src/create-component/FluentTheme.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
import { IFluentThemeShape, ColorRamp } from './FluentThemeShape';

export const FluentTheme: IFluentThemeShape = {
colors: {
brand: new ColorRamp(['#00f9ff', '#008e91', '#003233']),
neutral: new ColorRamp(['#dedede', '#7c7c7c', '#292929']),
},
typography: {
ramp: [8, 10, 12, 16, 24, 36, 48, 128],
fontFace: 'Futura',
},
};

export const FluentButtonTheme = {
styles: ({ typography, colors }: any) => ({
root: {
fontFamily: typography.fontFace,
fontSize: typography.ramp[5],
backgroundColor: colors.brand.strongest(),
color: colors.neutral.weakest(),
},
}),
variants: {
tiny: {
true: {
root: { fontSize: '20%' },
},
},
large: {
true: {
root: { fontSize: '400%' },
},
},
size: {
s: { root: { fontSize: '100%' } },
m: { root: { fontSize: '200%' } },
l: { root: { fontSize: '400%' } },
},
shadowed: {
true: { root: { fontSize: '77%', boxShadow: '10px 5px 5px purple' } },
},
bigIcon: {
true: {
root: { fontSize: '300%' },
icon: { fontSize: '300%' },
},
},
beautiful: {
true: props => ({
// bigIcon: true, size: m, shadowed: false
root: {
border: '3px solid pink',
},
}),
},
},
};

export const PlannerFluentTheme: IFluentThemeShape = {
colors: {
brand: new ColorRamp(['#00f9ff', '#008e91', '#003233']),
neutral: new ColorRamp(['#dedede', '#7c7c7c', '#292929']),
},
typography: {
ramp: [8, 10, 12, 16, 24, 36, 48, 128],
fontFace: 'Futura',
},
components: {
FluentButton: FluentButtonTheme,
FluentMenu: {
styles: () => ({
root: {
border: '1px solid red',
padding: '10px',
},
}),
variants: {
rounded: {
true: {
root: { borderRadius: '10px' },
},
},
},
},
FluentMenuItem: {
styles: () => ({
root: {
border: '1px solid blue',
},
}),
variants: {
rounded: {
true: {
root: { borderRadius: '20px' }, // FluentMenu should propagate this prop to the FluentMenuItem...
},
},
},
},
},
};
32 changes: 32 additions & 0 deletions packages/react-theming/src/create-component/FluentThemeShape.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { IBaseThemeShape } from './ThemeShape';

export class ColorRamp {
constructor(public colors: string[] = []) {}

public strongest(): string {
return this.colors[this.colors.length - 1];
}

public weakest(): string {
return this.colors[0];
}
}

export interface IFluentThemeShape extends IBaseThemeShape {
colors: {
brand: ColorRamp;
neutral: ColorRamp;
};

typography: {
ramp: number[];
fontFace: string;
};

components?: {
[key: string]: {
styles?: any;
variants?: any;
};
};
}
3 changes: 3 additions & 0 deletions packages/react-theming/src/create-component/ThemeShape.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export interface IBaseThemeShape {
components?: any;
}
256 changes: 256 additions & 0 deletions packages/react-theming/src/create-component/createComponent.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,256 @@
import { getClassName } from './createComponent';
import { ClassCache } from './ClassCache';

describe('createComponent', () => {
describe('getClassName', () => {
it('returns nothing in the default case', () => {
expect(getClassName(new ClassCache(), {}, {}, '')).toEqual({});
});

it('returns classNames for a single slot', () => {
expect(getClassName(new ClassCache(), {}, {}, '')).toEqual({});
});

it('returns customized classNames for a single slot', () => {
const cssRenderer = (args: any) => {
if (args.background === '#fff') {
return 'correct';
}
return 'incorrect';
};
expect(
getClassName(
new ClassCache(),
{
components: {
foo: {
variants: {
primary: {
true: {
root: {
background: '#fff',
},
},
},
},
},
},
},
{ primary: true },
'foo',
cssRenderer,
),
).toEqual({ root: 'correct' });
});

it('returns customized classNames for a single slot when multiple variants are specified', () => {
const cssRenderer = (args: any) => {
if (args.background === '#fff' && args.color === '#000') {
return 'correct';
}
return 'incorrect';
};
expect(
getClassName(
new ClassCache(),
{
components: {
foo: {
variants: {
primary: {
true: {
root: {
background: '#fff',
},
},
},
disabled: {
true: {
root: {
color: '#000',
},
},
},
},
},
},
},
{ primary: true, disabled: true },
'foo',
cssRenderer,
),
).toEqual({ root: 'correct' });
});

it('returns customized classNames for ennumerated variants', () => {
const cssRenderer = (args: any) => {
if (args.background === '#fff') {
return 'correct';
}
return 'incorrect';
};
expect(
getClassName(
new ClassCache(),
{
components: {
foo: {
variants: {
primary: {
very: {
root: {
background: '#fff',
},
},
},
},
},
},
},
{ primary: 'very' },
'foo',
cssRenderer,
),
).toEqual({ root: 'correct' });
});
});

describe('caching', () => {
it('uses the cache for a simple variant', () => {
let counter = 0;
const cssRenderer = (args: any) => `class-${counter++}`;
const theme = {
components: {
foo: {
variants: {
primary: {
true: {
root: {
background: '#fff',
},
},
},
},
},
},
};
const cache = new ClassCache();
const originalClassNames = getClassName(cache, theme, { primary: true }, 'foo', cssRenderer);
const nextRenderClassNames = getClassName(
cache,
theme,
{ primary: true },
'foo',
cssRenderer,
);
expect(nextRenderClassNames).toEqual(originalClassNames);
});

it('skips the cache for a separate theme', () => {
let counter = 0;
const cssRenderer = (args: any) => `class-${counter++}`;
const theme = {
components: {
foo: {
variants: {
primary: {
true: {
root: {
background: '#fff',
},
},
},
},
},
},
};
const anotherTheme = { ...theme };
const cache = new ClassCache();
const originalClassNames = getClassName(cache, theme, { primary: true }, 'foo', cssRenderer);
const nextRenderClassNames = getClassName(
cache,
anotherTheme,
{ primary: true },
'foo',
cssRenderer,
);
expect(nextRenderClassNames).not.toEqual(originalClassNames);
});
});

it('correctly merges variants', () => {
const cssRendererImportant = (args: any) => {
if (args.background === 'red') {
return 'correct';
}
return 'incorrect';
};
expect(
getClassName(
new ClassCache(),
{
components: {
foo: {
variants: {
primary: {
true: {
root: {
background: '#fff',
},
},
},
important: {
true: {
root: {
background: 'red',
},
},
},
},
},
},
},
{ primary: true, important: true },
'foo',
cssRendererImportant,
),
).toEqual({ root: 'correct' });

const cssRendererPrimary = (args: any) => {
if (args.background === '#fff') {
return 'correct';
}
return 'incorrect';
};
expect(
getClassName(
new ClassCache(),
{
components: {
foo: {
variants: {
important: {
true: {
root: {
background: 'red',
},
},
},
primary: {
true: {
root: {
background: '#fff',
},
},
},
},
},
},
},
{ important: true, primary: true },
'foo',
cssRendererPrimary,
),
).toEqual({ root: 'correct' });
});
});
144 changes: 144 additions & 0 deletions packages/react-theming/src/create-component/createComponent.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
import * as React from 'react';
import { ProviderContext } from '../components/ThemeProvider/Provider';
import { mergeCss } from '@uifabric/merge-styles';
import { VariantBasedCacheKeyStrategy, ClassCache } from './ClassCache';

// TODO:
// 1. how do we know the slots for component?
// 2. how do we tackle enum props (not just booleans)
// 3. final decision on styles living in theme (sync with JD)
// 4. type safety (props which are variants should be typed)
// 5. props which are variants should not be spreaded on the root
// 6. merging multiple variants styles should be predictable - maybe resolved...
// 7. merging is not correct (spreading is not enough, it should be deep merge)
// 8. How would it work for composition (Menu + MenuItem)
// 9. support inline-style calculation based on a prop
// 10. how to cache the styles

/**
* Solvable:
* 6, 9
*
* Possible blockers:
* P0: 3, 8, 10
* P1: 4, 5, 7
*
* Solved:
* 1, 2
*/

const getProps = (cssMap: any, props: any, slots: any = {}) => {
const newProps = {
...props,
slotProps: props.slotProps || {},
slots: { ...props.slots, ...slots },
};
Object.keys(cssMap).forEach(slotName => {
if (!newProps.slotProps[slotName]) {
newProps.slotProps[slotName] = {};
}
newProps.slotProps[slotName].className = `${newProps.slotProps[slotName].className ||
''} ${cssMap[slotName] || ''}`;
});

return newProps;
};

export const getClassName = (
cache: ClassCache,
theme: any,
componentProps: any,
componentName: string,
cssRenderer: (args: any) => string = mergeCss,
) => {
const stylesAdditions: any = {};
const variantNames: string[] = [];

const componentStyles =
Copy link
Contributor

Choose a reason for hiding this comment

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

Since TypeScript 3.7 introduced optional chaining you can use it here:
const componentStyles = theme?.components?.[componentName]?.styles ? ... : ...
Ref. https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-7.html

Copy link
Contributor

Choose a reason for hiding this comment

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

Same below for variants

theme &&
theme.components &&
theme.components[componentName] &&
theme.components[componentName].styles
? theme.components[componentName].styles({
typography: theme.typography,
colors: theme.colors,
})
: {};

const slotNames: string[] = Object.keys(componentStyles);

// We need to merge the slot names defined in the styles with the slot names
// defined in the variants
// styles = { [slot]: { css in js }
// variants = { [enumValue]: { [slot]: { css in js } } }
if (
theme &&
theme.components &&
theme.components[componentName] &&
theme.components[componentName].variants
) {
Object.keys(theme.components[componentName].variants).forEach(variantName => {
stylesAdditions[variantName] = {};
variantNames.push(variantName);
Object.keys(theme.components[componentName].variants[variantName]).forEach(enumValue => {
const variant: any = {};
stylesAdditions[variantName][enumValue] = variant;

Object.keys(theme.components[componentName].variants[variantName][enumValue]).forEach(
slotName => {
if (!slotNames.find(s => s === slotName)) {
slotNames.push(slotName);
}
variant[slotName] =
theme.components[componentName].variants[variantName][enumValue][slotName];
},
);
});
});
}

const mergedSlotStyles: any = {};

slotNames.forEach(slotName => {
mergedSlotStyles[slotName] = componentStyles[slotName] || {};
// eslint-disable-next-line array-callback-return
variantNames.map(v => {
if (
componentProps[v] !== undefined &&
stylesAdditions[v] !== undefined &&
stylesAdditions[v][componentProps[v]] !== undefined
) {
mergedSlotStyles[slotName] = {
...mergedSlotStyles[slotName],
...stylesAdditions[v][componentProps[v]][slotName],
};
}
});
});

const mutableCacheEntry: any = {};
const cacheKey = new VariantBasedCacheKeyStrategy(variantNames, componentProps);
const cacheEntry = cache.getOrSet(theme, cacheKey.toString(), mutableCacheEntry);

if (cacheEntry !== mutableCacheEntry) {
return cacheEntry;
}
slotNames.forEach(slotName => {
mutableCacheEntry[slotName] = cssRenderer(mergedSlotStyles[slotName]);
});
return mutableCacheEntry;
};

export const createComponent = (
displayName: string,
BaseComponent: any,
settings = { slots: {} },
) => {
const cache = new ClassCache();
return (props: any) => {
const theme = (React.useContext(ProviderContext) as any)!;
const cssMap = getClassName(cache, theme, props, displayName);
const newProps = getProps(cssMap, props, settings.slots);
return <BaseComponent {...newProps} />;
};
};
6 changes: 6 additions & 0 deletions packages/react-theming/src/index.ts
Original file line number Diff line number Diff line change
@@ -26,4 +26,10 @@ export { ThemeProvider } from './components/ThemeProvider/ThemeProvider';
export { Box } from './components/Box/Box';
export { createTheme } from './utilities/createTheme';

export { FluentButton } from './components/Button/FluentButton';
export { FluentMenu } from './components/Menu/FluentMenu';
export { FluentMenuItem } from './components/Menu/FluentMenuItem';
export { Provider } from './components/ThemeProvider/Provider';
export { FluentTheme, PlannerFluentTheme } from './create-component/FluentTheme';

jss.setup(preset());
2 changes: 2 additions & 0 deletions packages/react/src/utils/renderComponent.tsx
Original file line number Diff line number Diff line change
@@ -208,6 +208,8 @@ const renderComponent = <P extends {}>(
: {}

// Resolve styles using resolved variables, merge results, allow props.styles to override
// TODO: update mergeComponentStyles to cache its results based on theme object and prop combinations.
Copy link
Contributor Author

@mnajdova mnajdova Jan 7, 2020

Choose a reason for hiding this comment

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

@levithomason @jdhuntington regarding this comment, I made updates (added mergeComponentStylesWithCache) for caching.. This required to do some changes in the factories.ts (in the way of how the defaultProps, userProps and overrideProps' styles are merged (I am considering them all to be flat object for now - I know this is not ideal and may need some rethinking, but anyway, just sharing the current state. This was needed for the hashing to work...)

Let me know what you think..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Update - there are some perf gains with this:

Example with 1500 Menus, with different combinations of props, that repeatedly appear:

master avg: 5370.17
current avg: 4541.11

Chat with popover

master avg: 626.84
current avg: 596.07

ChatDuplicateMessages

master avg: 557.09 
current avg: 398.57

I had to add hash (id) on the theme - if it is not specified I am generating it as the JSON.stringify of the theme object (this is costly, is that's why for now I added hash on the different themes, but we can review it later). If it is useful I can move these changes to a separate PR and iterate on that.

Copy link
Member

Choose a reason for hiding this comment

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

Hm, I would expect this to be much faster. Are we sure it is working?

I added some comments on the cached function, have a look.

Copy link
Member

Choose a reason for hiding this comment

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

Also, it would be great to see the results of yarn perf on master as a baseline in this PR and another table with the current commit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here is a diff:

| Example                          | MASTER | CURRENT
|                                  | Avg    | Avg
| -------------------------------- | ------ | ------
| ProviderMergeThemes.perf.tsx     | 751.44 | 675.26
| DropdownManyItems.perf.tsx       | 527.11 | 484.49
| ChatWithPopover.perf.tsx         | 502.11 | 445.43
| ChatDuplicateMessages.perf.tsx   | 472.07 | 422.86
| CustomToolbar.perf.tsx           | 61.43  | 57.85
| SliderMinimal.perf.tsx           | 21.2   | 19.96
| ListCommon.perf.tsx              | 17.87  | 15.69
| EmbedMinimal.perf.tsx            | 17.29  | 15.68
| DropdownMinimal.perf.tsx         | 13.66  | 14.16
| AttachmentSlots.perf.tsx         | 11.98  | 10.02
| CarouselMinimal.perf.tsx         | 11.61  | 11.26
| SplitButtonMinimal.perf.tsx      | 10.27  | 8.88
| CheckboxMinimal.perf.tsx         | 8.68   | 7.43
| ProviderMinimal.perf.tsx         | 6.3    | 6.58
| LoaderMinimal.perf.tsx           | 5.97   | 6.02
| ButtonSlots.perf.tsx             | 5.18   | 5
| ReactionMinimal.perf.tsx         | 4.94   | 4.8
| InputMinimal.perf.tsx            | 4.6    | 4.15
| ButtonMinimal.perf.tsx           | 4.07   | 3.35
| AttachmentMinimal.perf.tsx       | 3.99   | 4.33
| TextAreaMinimal.perf.tsx         | 3.59   | 3.43
| DialogMinimal.perf.tsx           | 2.7    | 2.48
| AvatarMinimal.perf.tsx           | 2.5    | 2.27
| AlertMinimal.perf.tsx            | 2.37   | 2.3
| MenuMinimal.perf.tsx             | 2.3    | 2.16
| ItemLayoutMinimal.perf.tsx       | 2.12   | 1.97
| MenuButtonMinimal.perf.tsx       | 1.99   | 1.76
| LabelMinimal.perf.tsx            | 1.94   | 1.63
| AccordionMinimal.perf.tsx        | 1.83   | 1.62
| TreeMinimal.perf.tsx             | 1.69   | 1.37
| HeaderSlots.perf.tsx             | 1.63   | 1.63
| SegmentMinimal.perf.tsx          | 1.37   | 1.32
| VideoMinimal.perf.tsx            | 1.35   | 1.32
| HierarchicalTreeMinimal.perf.tsx | 1.35   | 1.39
| ChatMinimal.perf.tsx             | 1.29   | 1.05
| FormMinimal.perf.tsx             | 1.23   | 1.17
| ToolbarMinimal.perf.tsx          | 1.23   | 1.46
| DividerMinimal.perf.tsx          | 1.18   | 1.18
| TableMinimal.perf.tsx            | 1.05   | 0.97
| GridMinimal.perf.tsx             | 1.03   | 1.01
| IconMinimal.perf.tsx             | 0.95   | 0.97
| RadioGroupMinimal.perf.tsx       | 0.94   | 0.9
| StatusMinimal.perf.tsx           | 0.87   | 0.73
| PopupMinimal.perf.tsx            | 0.85   | 0.92
| ListMinimal.perf.tsx             | 0.8    | 0.67
| ChatMinimal.perf.tsx             | 1.29   | 1.05
| FlexMinimal.perf.tsx             | 0.71   | 0.68
| HeaderMinimal.perf.tsx           | 0.69   | 0.59
| ImageMinimal.perf.tsx            | 0.64   | 0.5
| LayoutMinimal.perf.tsx           | 0.62   | 0.61
| AnimationMinimal.perf.tsx        | 0.5    | 0.48
| TextMinimal.perf.tsx             | 0.47   | 0.5
| PortalMinimal.perf.tsx           | 0.37   | 0.36
| RefMinimal.perf.tsx              | 0.23   | 0.19
| BoxMinimal.perf.tsx              | 0.21   | 0.19

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please see comment: #2200 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

Cool, thanks for the diff. It looks like this isn't making a substantial improvement. Although many are a bit faster, some are actually slower too.

Let's see what happens with the immutable theme approach. Flamegrill can also help show you where cycles are being spent. You might want to compare approaches of hash/stringify with immutable and see where the time is going.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I opened PR: https://github.com/microsoft/fluent-ui-react/pull/2226/files and reverted the changes in this PR. Left some comments, take a look when you can, and we can move the discussion there.

// together with the already existent resolveStylesAndClasses caching, this should skip all style calculations on re-render.
const mergedStyles: ComponentSlotStylesPrepared = mergeComponentStyles(
theme.componentStyles[displayName],
withDebugId({ root: props.design }, 'props.design'),