diff --git a/.changeset/yummy-turkeys-search.md b/.changeset/yummy-turkeys-search.md new file mode 100644 index 0000000000..589c719ec2 --- /dev/null +++ b/.changeset/yummy-turkeys-search.md @@ -0,0 +1,6 @@ +--- +"@uifabricshared/foundation-settings": patch +"@fluentui-react-native/framework-base": patch +--- + +Stop caching styles automatically in mergeProps diff --git a/packages/deprecated/foundation-settings/etc/foundation-settings.api.md b/packages/deprecated/foundation-settings/etc/foundation-settings.api.md index e839799fd2..0b577ba9c4 100644 --- a/packages/deprecated/foundation-settings/etc/foundation-settings.api.md +++ b/packages/deprecated/foundation-settings/etc/foundation-settings.api.md @@ -4,7 +4,6 @@ ```ts // @public -export function flattenStyle(style: IStyleProp): object; // @public (undocumented) export function getActiveOverrides(target: IComponentSettings, lookup?: IOverrideLookup): string[]; @@ -60,7 +59,6 @@ export function mergeAndFinalizeSettings[]): object | undefined; // @public export function mergeProps(...props: (object | undefined)[]): TProps; diff --git a/packages/framework-base/src/component-patterns/slot.ts b/packages/framework-base/src/component-patterns/slot.ts index 4a20f0f4f0..0c10c796fb 100644 --- a/packages/framework-base/src/component-patterns/slot.ts +++ b/packages/framework-base/src/component-patterns/slot.ts @@ -2,6 +2,7 @@ import type React from 'react'; import type { PropsTransform, SlotComponent } from '../types/render.types'; import { SLOT_COMPONENT_KEY, SLOT_PROPS_KEY, SLOT_PROP_TRANSFORM_KEY } from '../const'; import { mergeProps } from '../merge-props/mergeProps'; +import { assignProps } from '../merge-props/assignProps'; /** * Convenience type, just referencing the statics of the component @@ -44,3 +45,12 @@ export function prepareSlotProps(slotInfo: SlotComponentStatics, const mergedProps = mergeProps(baseProps, userProps) ?? ({} as TProps); return transform ? transform(mergedProps) : mergedProps; } + +/** + * Attach the given props to the slot component statics object, merging them with any existing props. + * @param slot The slot component statics object to attach props to. + * @param props The props to attach to the slot component. + */ +export function attachSlotProps(slot: SlotComponentStatics, props: TProps): void { + slot[SLOT_PROPS_KEY] = assignProps((slot[SLOT_PROPS_KEY] ?? {}) as TProps, props); +} diff --git a/packages/framework-base/src/index.ts b/packages/framework-base/src/index.ts index 47bb7daa20..d4d5b8ced1 100644 --- a/packages/framework-base/src/index.ts +++ b/packages/framework-base/src/index.ts @@ -23,6 +23,7 @@ export { memoize } from './memo-cache/memoize'; */ export { mergeStyles } from './merge-props/mergeStyles'; export { mergeProps } from './merge-props/mergeProps'; +export { assignProps, assignStyles } from './merge-props/assignProps'; /** * ----- COMPONENT PATTERNS ----- @@ -31,6 +32,7 @@ export { mergeProps } from './merge-props/mergeProps'; export { renderSlot, createSlotComponent, renderJsx } from './component-patterns/render'; export { directComponent, legacyDirectComponent } from './component-patterns/direct'; export { phasedComponent, stagedComponent } from './component-patterns/phased'; +export { attachSlotProps } from './component-patterns/slot'; export { useSlot, useOptionalSlot } from './component-patterns/useSlot'; export { isDirectComponentType, diff --git a/packages/framework-base/src/merge-props/assignProps.test.ts b/packages/framework-base/src/merge-props/assignProps.test.ts new file mode 100644 index 0000000000..f80939182f --- /dev/null +++ b/packages/framework-base/src/merge-props/assignProps.test.ts @@ -0,0 +1,168 @@ +import { assignProps, assignStyles } from './assignProps'; +import type { StyleProp } from '../types/props.types'; + +interface IFakeStyle { + backgroundColor?: string; + color?: string; + fontFamily?: string; + borderWidth?: number; +} + +type IFakeStyleProp = StyleProp; +type IFakeProps = { a?: number; b?: number; style?: IFakeStyleProp }; + +describe('assignStyles', () => { + test('returns undefined when there are no valid styles', () => { + expect(assignStyles()).toBeUndefined(); + expect(assignStyles(undefined, null, false as unknown as IFakeStyleProp)).toBeUndefined(); + }); + + test('returns a single style object directly, preserving identity', () => { + const style: IFakeStyleProp = { color: 'red' }; + const result = assignStyles(style); + expect(result).toBe(style); + }); + + test('returns a single style directly even when other args are falsy', () => { + const style: IFakeStyleProp = { color: 'red' }; + const result = assignStyles(undefined, style, null); + expect(result).toBe(style); + }); + + test('returns a single array argument directly, preserving identity', () => { + const style: IFakeStyleProp = [{ color: 'red' }, { borderWidth: 1 }]; + const result = assignStyles(style); + expect(result).toBe(style); + }); + + test('combines multiple style objects into a flat array without flattening values', () => { + const a: IFakeStyleProp = { color: 'red' }; + const b: IFakeStyleProp = { backgroundColor: 'blue' }; + const result = assignStyles(a, b) as IFakeStyle[]; + expect(result).toEqual([a, b]); + expect(result[0]).toBe(a); + expect(result[1]).toBe(b); + }); + + test('flattens nested arrays into a single flat array', () => { + const a: IFakeStyleProp = { color: 'red' }; + const b: IFakeStyleProp = { backgroundColor: 'blue' }; + const c: IFakeStyleProp = { borderWidth: 2 }; + const result = assignStyles([a], [[b], c]) as IFakeStyle[]; + expect(result).toEqual([a, b, c]); + }); + + test('drops falsy entries while combining multiple styles', () => { + const a: IFakeStyleProp = { color: 'red' }; + const b: IFakeStyleProp = { backgroundColor: 'blue' }; + const result = assignStyles(a, [null, undefined, b, false as unknown as IFakeStyleProp]) as IFakeStyle[]; + expect(result).toEqual([a, b]); + }); + + test('does not mutate the input styles', () => { + const a: IFakeStyleProp = [{ color: 'red' }]; + const b: IFakeStyleProp = { backgroundColor: 'blue' }; + assignStyles(a, b); + expect(a).toEqual([{ color: 'red' }]); + expect((a as IFakeStyle[]).length).toBe(1); + }); +}); + +describe('assignProps', () => { + test('mutates and returns the base object', () => { + const base = { a: 1 }; + const result = assignProps(base, { b: 2 } as Partial); + expect(result).toBe(base); + expect(result).toEqual({ a: 1, b: 2 }); + }); + + test('later props override earlier props (last-wins)', () => { + const base = { a: 1, b: 1 }; + const result = assignProps(base, { b: 2 }, { b: 3 }); + expect(result).toEqual({ a: 1, b: 3 }); + }); + + test('returns a fresh object when the base is not an object', () => { + const result = assignProps(null as unknown as { a?: number }, { a: 1 }); + expect(result).toEqual({ a: 1 }); + expect(result).not.toBeNull(); + }); + + test('returns a fresh object when the base is undefined', () => { + const result = assignProps(undefined as unknown as { a?: number }, { a: 1 }, { b: 2 } as { a?: number }); + expect(result).toEqual({ a: 1, b: 2 }); + expect(result).toBeInstanceOf(Object); + }); + + test('ignores undefined and null entries in the rest arguments', () => { + const base: IFakeProps = { a: 1 }; + const result = assignProps(base, undefined as unknown as Partial, { b: 2 }, null as unknown as Partial); + expect(result).toBe(base); + expect(result).toEqual({ a: 1, b: 2 }); + }); + + test('merges styles from base and rest into an array rather than replacing', () => { + const baseStyle = { color: 'red' }; + const restStyle = { backgroundColor: 'blue' }; + const base = { style: baseStyle as IFakeStyleProp }; + const result = assignProps(base, { style: restStyle as IFakeStyleProp }); + expect(result.style).toEqual([baseStyle, restStyle]); + expect(result).toBe(base); + }); + + test('keeps a single style intact when only the base has a style', () => { + const baseStyle = { color: 'red' }; + const base = { style: baseStyle as IFakeStyleProp, a: 1 }; + const result = assignProps(base, { a: 2 } as Partial); + expect(result.style).toBe(baseStyle); + expect(result.a).toBe(2); + }); + + test('applies a style supplied only in rest', () => { + const restStyle = { color: 'green' }; + const base: IFakeProps = { a: 1 }; + const result = assignProps(base, { style: restStyle as IFakeStyleProp }); + expect(result.style).toBe(restStyle); + expect(result.a).toBe(1); + }); + + test('merged style wins over individually assigned style props', () => { + const baseStyle = { color: 'red' }; + const restStyle = { color: 'blue' }; + const base = { style: baseStyle as IFakeStyleProp }; + const result = assignProps(base, { style: restStyle as IFakeStyleProp }); + // the combined array is applied last, so it is not clobbered by the raw rest style + expect(result.style).toEqual([baseStyle, restStyle]); + }); + + test('combines styles from base and multiple rest props in order', () => { + const baseStyle = { color: 'red' }; + const restStyle1 = { backgroundColor: 'blue' }; + const restStyle2 = { borderWidth: 2 }; + const base = { style: baseStyle as IFakeStyleProp }; + const result = assignProps(base, { style: restStyle1 as IFakeStyleProp }, { style: restStyle2 as IFakeStyleProp }); + expect(result.style).toEqual([baseStyle, restStyle1, restStyle2]); + }); + + test('flattens array-valued styles when combining base and rest', () => { + const baseStyle = [{ color: 'red' }, { borderWidth: 1 }]; + const restStyle = { backgroundColor: 'blue' }; + const base: IFakeProps = { style: baseStyle }; + const result = assignProps(base, { style: restStyle }); + expect(result.style).toEqual([{ color: 'red' }, { borderWidth: 1 }, { backgroundColor: 'blue' }]); + }); + + test('does not mutate the rest props objects', () => { + const rest = { b: 2 }; + const base = { a: 1 } as { a: number; b?: number }; + assignProps(base, rest); + expect(rest).toEqual({ b: 2 }); + }); + + test('leaves the base untouched when no rest props are provided', () => { + const base = { a: 1 }; + const result = assignProps(base); + expect(result).toBe(base); + expect(result).toEqual({ a: 1 }); + }); +}); diff --git a/packages/framework-base/src/merge-props/assignProps.ts b/packages/framework-base/src/merge-props/assignProps.ts new file mode 100644 index 0000000000..37bbb2e82d --- /dev/null +++ b/packages/framework-base/src/merge-props/assignProps.ts @@ -0,0 +1,91 @@ +import type { StyleProp } from 'react-native'; +import { isObject } from '../utilities/typeUtils'; + +/** + * This will combine multiple styles together. Instead of fully flattening it will keep the style objects intact but build them + * up into a single flat array with all the individual style values. + * @param base the base style prop to start with, if this is an array, elements will be added to that array, otherwise a new array will be created + * @param styles the additional style props to combine with the base style, can be arrays or individual style objects + * @returns the combined style prop, which will in most cases be an array containing all the individual style values from the base and additional styles + */ +export function assignStyles(...styles: StyleProp[]): StyleProp { + const toApply: StyleProp[] = styles.filter(Boolean); + if (toApply.length === 0) { + return undefined; + } else if (toApply.length === 1) { + return toApply[0]; + } + return assignStylesWorker([], toApply); +} + +/** + * This function merges props together with the same use pattern as Object.assign, except that it will ensure + * styles get merged rather than replaced. + * + * @param base The base props object to start with. This will be the object that receives the merged properties. + * @param rest Additional props objects whose properties will be merged into the base object. + * @returns The resulting props object with all properties from the base and additional props merged together. + */ +export function assignProps(base: TProps, ...rest: Partial[]): TProps { + // get the base object to add values to + const result = isObject(base) ? base : {}; + + // collect all styles that will need merging, and do the merge upfront + const stylesToMerge = [getStyle(base), ...rest.map((props) => getStyle(props))].filter(Boolean); + const style = assignStyles(...stylesToMerge); + if (style) { + // if we have a merged style, add it to the rest of the props so it will be included in the final Object.assign + rest.push({ style } as unknown as Partial); + } + // finally, assign all the rest of the props to the result object + return trimUndefinedKeys(Object.assign(result, ...rest)); +} + +/** + * Extracts the style property from a props object if it exists. + * + * @param props The props object to extract the style from. + * @returns The style property if it exists, otherwise undefined. + */ +function getStyle(props: unknown): StyleProp | undefined { + if (isObject(props) && 'style' in props) { + return (props as { style?: StyleProp }).style; + } + return undefined; +} + +/** + * Recursively collects style objects into a single array, preserving the structure of nested arrays. + * + * @param collector The array that collects individual style objects. + * @param styles The style prop to process, which can be an array or a single style object. + * @returns The collector array containing all individual style objects. + */ +function assignStylesWorker(collector: StyleProp[], styles: StyleProp): StyleProp { + if (styles) { + if (Array.isArray(styles)) { + for (const style of styles) { + assignStylesWorker(collector, style); + } + } else { + collector.push(styles); + } + } + return collector; +} + +/** + * Remove all keys with undefined values from the given object. + * @param obj The object from which undefined keys should be removed. + * @returns The same object with all undefined keys removed. + */ +export function trimUndefinedKeys(obj: T): T { + if (isObject(obj)) { + for (const key in obj) { + if (obj[key] === undefined) { + delete obj[key]; + } + } + } + return obj; +} diff --git a/packages/framework-base/src/merge-props/mergeStyles.test.ts b/packages/framework-base/src/merge-props/mergeStyles.test.ts index 93e1be336b..d1552b3fc3 100644 --- a/packages/framework-base/src/merge-props/mergeStyles.test.ts +++ b/packages/framework-base/src/merge-props/mergeStyles.test.ts @@ -96,30 +96,33 @@ describe('Style flatten and merge tests', () => { expect(merged).toEqual(sMergedSelectors); }); - test('memo recursive arrays', () => { + test('merge recursive arrays does not maintain object identity', () => { const flattened = mergeStyles(s1); const flattened2 = mergeStyles(s1); expect(flattened).toEqual(s1flatten); - expect(flattened2).toBe(flattened); + expect(flattened2).toEqual(flattened); + expect(flattened2).not.toBe(flattened); }); - test('memo flat style', () => { + test('merge flat style returns the style directly', () => { const flattened = mergeStyles(s2); const flattened2 = mergeStyles(s2); expect(flattened).toBe(s2); - expect(flattened2).toBe(flattened); + expect(flattened2).toBe(s2); }); - test('memo and flatten multiple', () => { + test('merge and flatten multiple does not maintain object identity', () => { const flattened = mergeStyles(s1, s2); const flattened2 = mergeStyles(s1, s2); expect(flattened).toEqual(sMerged); - expect(flattened2).toBe(flattened); + expect(flattened2).toEqual(flattened); + expect(flattened2).not.toBe(flattened); }); - test('memo styles ignores undefined values', () => { + test('merge styles ignores undefined values', () => { const result1 = mergeStyles(s1, s2, undefined, s1flatten); const result2 = mergeStyles(s1, undefined, s2, s1flatten); - expect(result2).toBe(result1); + expect(result2).toEqual(result1); + expect(result2).not.toBe(result1); }); }); diff --git a/packages/framework-base/src/merge-props/mergeStyles.ts b/packages/framework-base/src/merge-props/mergeStyles.ts index bf7bb20199..ae465f42cf 100644 --- a/packages/framework-base/src/merge-props/mergeStyles.ts +++ b/packages/framework-base/src/merge-props/mergeStyles.ts @@ -1,5 +1,4 @@ import { immutableMerge } from '../immutable-merge/Merge'; -import { getMemoCache } from '../memo-cache/getMemoCache'; import type { StyleMerger, StyleProp } from '../types/props.types'; /** @@ -28,8 +27,6 @@ export const mergeAndFlattenStyles: StyleMerger = (...styles: StyleProp ); }; -const _styleCache = getMemoCache(); - /** * Function overloads to allow merging styles of different types. * This is useful when merging token-based styles with React Native StyleProp types. @@ -40,6 +37,6 @@ export const mergeStyles: StyleMerger = (...styles: StyleProp[]) => { // now memo the results if there is more than one element or if the one element is an array return inputs.length > 1 || (inputs.length === 1 && Array.isArray(inputs[0])) - ? _styleCache(() => mergeAndFlattenStyles(undefined, ...inputs), inputs)[0] + ? mergeAndFlattenStyles(undefined, ...inputs) : inputs[0] || {}; }; diff --git a/packages/framework-base/targets/tsconfig.check.json b/packages/framework-base/targets/tsconfig.check.json index 3f1ff13cff..c32bd0039f 100644 --- a/packages/framework-base/targets/tsconfig.check.json +++ b/packages/framework-base/targets/tsconfig.check.json @@ -1,7 +1,7 @@ { "extends": "@fluentui-react-native/scripts/tsconfig-strict", "compilerOptions": { - "rootDir": "../src", + "rootDir": "..", "noEmit": true, "allowJs": true, "checkJs": true,