-
Notifications
You must be signed in to change notification settings - Fork 55
Conversation
packages/react/src/types.ts
Outdated
@@ -168,4 +168,5 @@ export interface ProviderContextPrepared { | |||
target: Document | |||
theme: ThemePrepared | |||
originalThemes: (ThemeInput | undefined)[] | |||
resolvedComponentVariables: Record<string, object> |
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.
resolvedComponentVariables: Record<string, object> | |
DO_NOT_USE_OR_YOU_WILL_BE_FIRED_resolvedComponentVariables: Record<string, object> |
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.
would _internal_resolvedComponentVariables
be enough?
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.
renamed
props.variables && withDebugId(props.variables, 'props.variables'), | ||
)(theme.siteVariables) | ||
// Resolve variables for this component, cache the result in provider | ||
if (!(displayName in resolvedComponentVariables)) { |
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.
if (!(displayName in resolvedComponentVariables)) { | |
if (!resolvedComponentVariables[displayName]) { |
If there difference between this?
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.
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.
removed
resolvedComponentVariables[displayName], | ||
withDebugId(props.variables, 'props.variables'), | ||
)(theme.siteVariables) | ||
: resolvedComponentVariables[displayName] |
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.
nit, it's hard to read this, I suggest
let resolvedVariables = resolvedComponentVariables[displayName]
if (props.variables) {
resolvedVariables = mergeComponentVariables(
resolvedComponentVariables[displayName],
withDebugId(props.variables, 'props.variables'),
)(theme.siteVariables)
}
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.
I don't like let
.
Do not resolve component variables in each render.
Resolved variables are now cached in
ProviderContext
. NestedProvider
starts with an empty cache.Performance: