fix(useStateManager): keep reference to actions

CHANGELOG.md

@@ -27,6 +27,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - Fix page crash when `Tooltip` content is null @delprzemo ([#2332](https://github.com/microsoft/fluent-ui-react/pull/2332))
 - Fix `document` usage in `mergeProviderContexts` to get SSR working @layershifter ([#2330](https://github.com/microsoft/fluent-ui-react/pull/2330))
 - Fix `input` descenders being cropped in the Teams theme @bcalvery ([#2335](https://github.com/microsoft/fluent-ui-react/pull/2335))
+- Use referentially equal objects for `actions` in `useStateManager` @layershifter ([#2347](https://github.com/microsoft/fluent-ui-react/pull/2347))
 
 ### Features
 - Added sourcemaps to the dist output to simplify debugging @miroslavstastny ([#2329](https://github.com/microsoft/fluent-ui-react/pull/2329))

packages/react-bindings/src/hooks/useStateManager.ts

@@ -36,6 +36,7 @@ const useStateManager = <
     mapPropsToState = () => ({} as Partial<State>),
     sideEffects = [],
   } = options
+  const latestActions = React.useMemo<Actions>(() => ({} as Actions), [managerFactory])
   const latestManager = React.useRef<Manager<State, Actions> | null>(null)
 
   // Heads up! forceUpdate() is used only for triggering rerenders, stateManager is SSOT
@@ -58,19 +59,28 @@ const useStateManager = <
     ],
   })
 
+  // We need to keep the same reference to an object with actions to allow usage them as
+  // a dependency in useCallback() hook
+  Object.assign(latestActions, latestManager.current.actions)
+
+  // For development environments we disallow ability to extend object with other properties to
+  // avoid misusage
+  if (process.env.NODE_ENV !== 'production') {
+    if (Object.isExtensible(latestActions)) Object.preventExtensions(latestActions)
+  }
+
   // We need to pass exactly `manager.state` to provide the same state object during the same render
   // frame.
   // It keeps behavior consistency between React state tools and our managers
   // https://github.com/facebook/react/issues/11527#issuecomment-360199710
-
-  if (process.env.NODE_ENV === 'production') {
-    return { state: latestManager.current.state, actions: latestManager.current.actions }
-  }
-
   // Object.freeze() is used only in dev-mode to avoid usage mistakes
+
   return {
-    state: Object.freeze(latestManager.current.state),
-    actions: Object.freeze(latestManager.current.actions),
+    state:
+      process.env.NODE_ENV === 'production'
+        ? latestManager.current.state
+        : Object.freeze(latestManager.current.state),
+    actions: latestActions,
   }
 }
 

packages/react-bindings/test/hooks/useStateManager-test.tsx

@@ -1,6 +1,6 @@
 import { useStateManager } from '@fluentui/react-bindings'
 import { createManager, ManagerFactory } from '@fluentui/state'
-import { shallow } from 'enzyme'
+import { mount, shallow } from 'enzyme'
 import * as React from 'react'
 import * as ReactTestUtils from 'react-dom/test-utils'
 
@@ -63,6 +63,23 @@ const TestComponent: React.FunctionComponent<TestComponentProps> = props => {
   )
 }
 
+type ActionsComponentProps = {
+  onRender: () => void
+  onUpdate: () => void
+}
+
+const ActionsComponent: React.FunctionComponent<ActionsComponentProps> = props => {
+  const { actions, state } = useStateManager(createTestManager)
+  const handleClick = React.useCallback(() => actions.toggle(), [actions])
+
+  props.onRender()
+  React.useEffect(() => {
+    props.onUpdate()
+  }, [actions])
+
+  return <div data-open={state.open} onClick={handleClick} />
+}
+
 describe('useStateManager', () => {
   it('uses default values from state manager', () => {
     const wrapper = shallow(<TestComponent />)
@@ -148,4 +165,30 @@ describe('useStateManager', () => {
     expect(onChange).toHaveBeenNthCalledWith(1, 'foo')
     expect(onChange).toHaveBeenNthCalledWith(2, 'foo')
   })
+
+  it('actions are referentially equal between renders', () => {
+    const onRender = jest.fn()
+    const onUpdate = jest.fn()
+    const wrapper = mount(<ActionsComponent onRender={onRender} onUpdate={onUpdate} />)
+
+    expect(wrapper.find('div').prop('data-open')).toBe(false)
+
+    ReactTestUtils.act(() => {
+      wrapper.find('div').simulate('click')
+    })
+    wrapper.update()
+
+    expect(wrapper.find('div').prop('data-open')).toBe(true)
+    expect(onRender).toHaveBeenCalledTimes(2)
+    expect(onUpdate).toHaveBeenCalledTimes(1)
+
+    ReactTestUtils.act(() => {
+      wrapper.find('div').simulate('click')
+    })
+    wrapper.update()
+
+    expect(wrapper.find('div').prop('data-open')).toBe(false)
+    expect(onRender).toHaveBeenCalledTimes(3)
+    expect(onUpdate).toHaveBeenCalledTimes(1)
+  })
 })