-
Notifications
You must be signed in to change notification settings - Fork 133
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve useRestyle performance #131
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,7 @@ | ||
import {useMemo} from 'react'; | ||
import {StyleProp} from 'react-native'; | ||
import {StyleProp, ViewStyle, TextStyle, ImageStyle} from 'react-native'; | ||
|
||
import {BaseTheme, RestyleFunctionContainer, RNStyle} from '../types'; | ||
import composeRestyleFunctions from '../composeRestyleFunctions'; | ||
import {BaseTheme, RNStyle, Dimensions} from '../types'; | ||
import {getKeys} from '../typeHelpers'; | ||
|
||
import useDimensions from './useDimensions'; | ||
|
@@ -13,24 +12,20 @@ const filterRestyleProps = < | |
TProps extends Record<string, unknown> & TRestyleProps | ||
>( | ||
props: TProps, | ||
omitList: (keyof TRestyleProps)[], | ||
): Omit<TProps, keyof TRestyleProps> => { | ||
const omittedProp = omitList.reduce<Record<keyof TRestyleProps, boolean>>( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Every time we were going to filter the props to distinguish between restyle props and non restyle props, we would build this map. Now, we build the map only once when calling |
||
(acc, prop) => { | ||
acc[prop] = true; | ||
return acc; | ||
}, | ||
{} as Record<keyof TRestyleProps, boolean>, | ||
); | ||
|
||
omitPropertiesMap: Record<keyof TProps, boolean>, | ||
) => { | ||
return getKeys(props).reduce( | ||
(acc, key) => { | ||
if (!omittedProp[key as keyof TRestyleProps]) { | ||
acc[key] = props[key]; | ||
({cleanProps, restyleProps}, key) => { | ||
if (omitPropertiesMap[key as keyof TProps]) { | ||
return {cleanProps, restyleProps: {...restyleProps, [key]: props[key]}}; | ||
} else { | ||
return {cleanProps: {...cleanProps, [key]: props[key]}, restyleProps}; | ||
} | ||
return acc; | ||
}, | ||
{} as TProps, | ||
{cleanProps: {}, restyleProps: {}} as { | ||
cleanProps: TProps; | ||
restyleProps: TRestyleProps; | ||
}, | ||
); | ||
}; | ||
|
||
|
@@ -39,28 +34,39 @@ const useRestyle = < | |
TRestyleProps extends Record<string, any>, | ||
TProps extends TRestyleProps & {style?: StyleProp<RNStyle>} | ||
>( | ||
restyleFunctions: ( | ||
| RestyleFunctionContainer<TProps, Theme> | ||
| RestyleFunctionContainer<TProps, Theme>[])[], | ||
composedRestyleFunction: { | ||
buildStyle: <TInputProps extends TProps>( | ||
props: TInputProps, | ||
{ | ||
theme, | ||
dimensions, | ||
}: { | ||
theme: Theme; | ||
dimensions: Dimensions; | ||
}, | ||
) => ViewStyle | TextStyle | ImageStyle; | ||
properties: (keyof TProps)[]; | ||
propertiesMap: Record<keyof TProps, boolean>; | ||
}, | ||
props: TProps, | ||
) => { | ||
const theme = useTheme<Theme>(); | ||
|
||
const dimensions = useDimensions(); | ||
|
||
const restyled = useMemo(() => { | ||
const composedRestyleFunction = composeRestyleFunctions(restyleFunctions); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
We can do that only once when calling |
||
const style = composedRestyleFunction.buildStyle(props, { | ||
const {cleanProps, restyleProps} = filterRestyleProps( | ||
props, | ||
composedRestyleFunction.propertiesMap, | ||
); | ||
const style = composedRestyleFunction.buildStyle(restyleProps, { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We were calling |
||
theme, | ||
dimensions, | ||
}); | ||
const cleanProps = filterRestyleProps( | ||
props, | ||
composedRestyleFunction.properties, | ||
); | ||
(cleanProps as TProps).style = [style, props.style].filter(Boolean); | ||
|
||
cleanProps.style = [style, props.style].filter(Boolean); | ||
return cleanProps; | ||
}, [restyleFunctions, props, dimensions, theme]); | ||
}, [composedRestyleFunction, props, dimensions, theme]); | ||
|
||
return restyled; | ||
}; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Each style function in
funcs
takes all the props of the component, applies a transformation and returns a style attribute that then will be used to create a stylesheet. If there is no value to transform, each function returns an empty object instead.Here, we were running every style function every time even though, not all transformations were needed. Example:
<Box marginTop="m" />
Here we only need to transform
marginTop="m"
tomarginTop: 16
, however we run all the transformations registered for the Box component here: https://github.com/sbalay/restyle-benchmark/blob/master/src/restyle/createBox.ts#L47So we also check for
backgroundColor
and return an empty object, we also check formarginBottom
and returns an empty object, we also check forpaddingHorizontal
and so on.With the change below, we only run transformation functions for the props that are present. In the example above we would only run the function that transforms
marginTop="m"
to{marginTop: 16}