Skip to content

Commit 406e931

Browse files
natewnatew
committed
fix(core): fix bug when removing group style while group style state is set not clearing group state
1 parent 1eb40c4 commit 406e931

File tree

4 files changed

+51
-31
lines changed

4 files changed

+51
-31
lines changed

‎code/core/animations-motion/src/createAnimations.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,7 @@ export function createAnimations<A extends Record<string, AnimationConfig>>(
325325
animationOptions,
326326
isExiting,
327327
isFirstRender: isFirstRender.current,
328+
animationProps,
328329
})
329330
console.groupEnd()
330331
}

‎code/core/web/src/createComponent.tsx‎

Lines changed: 37 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import { getShorthandValue } from './helpers/getShorthandValue'
2121
import { getSplitStyles, useSplitStyles } from './helpers/getSplitStyles'
2222
import { log } from './helpers/log'
2323
import { mergeProps } from './helpers/mergeProps'
24+
import { objectIdentityKey } from './helpers/objectIdentityKey'
2425
import { setElementProps } from './helpers/setElementProps'
2526
import { subscribeToContextGroup } from './helpers/subscribeToContextGroup'
2627
import { themeable } from './helpers/themeable'
@@ -425,9 +426,8 @@ export function createComponent<
425426
return groupContextParent
426427
}
427428

428-
// TODO this shouldn't be in useMemo
429-
stateRef.current.group?.listeners.clear()
430429
const listeners = new Set<GroupStateListener>()
430+
stateRef.current.group?.listeners?.clear()
431431
stateRef.current.group = {
432432
listeners,
433433
emit(state) {
@@ -635,14 +635,16 @@ export function createComponent<
635635
debugProp
636636
)
637637

638+
const isPassthrough = !splitStyles
639+
638640
// splitStyles === null === passThrough
639641

640642
const groupContext = groupName ? allGroupContexts?.[groupName] || null : null
641643

642644
// one tiny mutation 🙏 get width/height optimistically from raw values if possible
643645
// if set hardcoded it avoids extra renders
644646
if (
645-
splitStyles &&
647+
!isPassthrough &&
646648
groupContext &&
647649
// avoids onLayout if we don't need it
648650
props.containerType !== 'normal'
@@ -661,7 +663,7 @@ export function createComponent<
661663
// avoids re-rendering if animation driver supports it
662664
// TODO believe we need to set some sort of "pendingState" in case it re-renders
663665
if (
664-
splitStyles &&
666+
!isPassthrough &&
665667
(hasAnimationProp || groupName) &&
666668
animationDriver?.avoidReRenders
667669
) {
@@ -670,22 +672,6 @@ export function createComponent<
670672
stateRef.current.updateStyleListener = () => {
671673
const updatedState = NextState.get(stateRef) || state
672674
const mediaState = NextMedia.get(stateRef)
673-
const {
674-
group,
675-
hasDynGroupChildren,
676-
unmounted,
677-
animation,
678-
...childrenGroupState
679-
} = updatedState
680-
681-
// update before getting styles
682-
if (groupContext) {
683-
notifyGroupSubscribers(
684-
groupContext,
685-
stateRef.current.group || null,
686-
childrenGroupState
687-
)
688-
}
689675

690676
const nextStyles = getSplitStyles(
691677
props,
@@ -707,6 +693,24 @@ export function createComponent<
707693
useStyleListener?.((nextStyles?.style || {}) as any)
708694
}
709695

696+
function updateGroupListeners() {
697+
const updatedState = NextState.get(stateRef)!
698+
if (groupContext) {
699+
const {
700+
group,
701+
hasDynGroupChildren,
702+
unmounted,
703+
animation,
704+
...childrenGroupState
705+
} = updatedState
706+
notifyGroupSubscribers(
707+
groupContext,
708+
stateRef.current.group || null,
709+
childrenGroupState
710+
)
711+
}
712+
}
713+
710714
// don't change this ever or else you break ComponentContext and cause re-rendering
711715
componentContext.mediaEmit ||= (next) => {
712716
NextMedia.set(stateRef, next)
@@ -739,11 +743,15 @@ export function createComponent<
739743
debugProp &&
740744
debugProp !== 'profile'
741745
) {
742-
console.groupCollapsed(`[⚡️] avoid setState`, next, { updatedState, props })
746+
console.groupCollapsed(`[⚡️] avoid setState`, componentName, next, {
747+
updatedState,
748+
props,
749+
})
743750
console.info(stateRef.current.host)
744751
console.groupEnd()
745752
}
746753

754+
updateGroupListeners()
747755
stateRef.current.updateStyleListener?.()
748756
} else {
749757
if (
@@ -917,7 +925,7 @@ export function createComponent<
917925
if (process.env.NODE_ENV === 'development' && time) time`destructure`
918926

919927
if (
920-
splitStyles &&
928+
!isPassthrough &&
921929
groupContext && // avoids onLayout if we don't need it
922930
props.containerType !== 'normal'
923931
) {
@@ -1036,6 +1044,7 @@ export function createComponent<
10361044

10371045
useIsomorphicLayoutEffect(() => {
10381046
if (disabled) return
1047+
10391048
if (!pseudoGroups && !mediaGroups) return
10401049
if (!allGroupContexts) return
10411050
return subscribeToContextGroup({
@@ -1047,13 +1056,14 @@ export function createComponent<
10471056
}, [
10481057
allGroupContexts,
10491058
disabled,
1050-
pseudoGroups ? Object.keys([...pseudoGroups]).join('') : 0,
1051-
mediaGroups ? Object.keys([...mediaGroups]).join('') : 0,
1059+
pseudoGroups ? objectIdentityKey(pseudoGroups) : 0,
1060+
mediaGroups ? objectIdentityKey(mediaGroups) : 0,
10521061
])
10531062

10541063
const groupEmitter = stateRef.current.group
10551064
useIsomorphicLayoutEffect(() => {
10561065
if (!groupContext || !groupEmitter) return
1066+
10571067
notifyGroupSubscribers(groupContext, groupEmitter, state)
10581068
}, [groupContext, groupEmitter, state])
10591069

@@ -1299,8 +1309,7 @@ export function createComponent<
12991309

13001310
if (process.env.NODE_ENV === 'development' && time) time`spaced-as-child`
13011311

1302-
// passthrough mode - only pass style display contents, nothing else
1303-
if (!splitStyles) {
1312+
if (isPassthrough) {
13041313
content = propsIn.children
13051314
elementType = BaseViewComponent
13061315
viewProps = {
@@ -1379,7 +1388,7 @@ export function createComponent<
13791388
content = (
13801389
<span
13811390
className="_dsp_contents"
1382-
{...(splitStyles && isHydrated && events && getWebEvents(events))}
1391+
{...(!isPassthrough && isHydrated && events && getWebEvents(events))}
13831392
>
13841393
{content}
13851394
</span>
@@ -1413,7 +1422,7 @@ export function createComponent<
14131422
if (process.env.TAMAGUI_TARGET === 'web' && startedUnhydrated) {
14141423
// breaking rules of hooks but startedUnhydrated NEVER changes
14151424
const styleTags = useMemo(() => {
1416-
if (!splitStyles) return
1425+
if (isPassthrough) return
14171426
return getStyleTags(Object.values(splitStyles.rulesToInsert))
14181427
}, [])
14191428
// this is only to appease react hydration really

‎code/core/web/src/helpers/getSplitStyles.tsx‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -948,16 +948,18 @@ export const getSplitStyles: StyleSplitter = (
948948

949949
// $group-x
950950
const groupState = groupContext?.[groupName]?.state
951+
const groupPseudoKey = groupInfo.pseudo
952+
const groupMediaKey = groupInfo.media
951953

952954
if (!groupState) {
953955
if (process.env.NODE_ENV === 'development' && debug) {
954956
log(`No parent with group prop, skipping styles: ${groupName}`)
955957
}
958+
// we still want to indicate we should listen! this is how subscribeToGroupContext knows to run
959+
pseudoGroups ||= new Set()
956960
return
957961
}
958962

959-
const groupPseudoKey = groupInfo.pseudo
960-
const groupMediaKey = groupInfo.media
961963
const componentGroupState = componentState.group?.[groupName]
962964

963965
if (groupMediaKey) {

‎code/core/web/src/helpers/subscribeToContextGroup.tsx‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ const createGroupListener = (
5151
return () => {}
5252
}
5353

54-
return parent.subscribe(({ layout, pseudo }) => {
54+
const dispose = parent.subscribe(({ layout, pseudo }) => {
5555
setStateShallow((prev) => {
5656
let didChange = false
5757
const group = prev.group?.[name] || {
@@ -88,4 +88,12 @@ const createGroupListener = (
8888
return prev
8989
})
9090
})
91+
92+
return () => {
93+
dispose()
94+
// we no longer have any active group, need to remove state so the style updates
95+
setStateShallow({
96+
group: {},
97+
})
98+
}
9199
}

0 commit comments

Comments
 (0)