fix(ToggleSwitch): fire onChange from the interaction instead of an effect

.changeset/toggleswitch-onchange-from-handler.md

@@ -0,0 +1,5 @@
+---
+'@primer/react': patch
+---
+
+ToggleSwitch: Fire `onChange` from the user interaction instead of an effect, so it no longer fires on mount or when a controlled `checked` value changes externally.

packages/react/src/ToggleSwitch/ToggleSwitch.test.tsx

@@ -181,6 +181,46 @@ describe('ToggleSwitch', () => {
     expect(handleChange).toHaveBeenCalledWith(true)
   })
 
+  it('does not call onChange on mount or when checked changes externally', () => {
+    const handleChange = vi.fn()
+    const {rerender} = render(
+      <>
+        <div id="switchLabel">{SWITCH_LABEL_TEXT}</div>
+        <ToggleSwitch checked={false} onChange={handleChange} aria-labelledby="switchLabel" />
+      </>,
+    )
+    expect(handleChange).not.toHaveBeenCalled()
+
+    rerender(
+      <>
+        <div id="switchLabel">{SWITCH_LABEL_TEXT}</div>
+        <ToggleSwitch checked={true} onChange={handleChange} aria-labelledby="switchLabel" />
+      </>,
+    )
+    expect(handleChange).not.toHaveBeenCalled()
+  })
+
+  it('does not call onChange when a new inline onChange is passed on rerender', () => {
+    const handleChange = vi.fn()
+    const {rerender} = render(
+      <>
+        <div id="switchLabel">{SWITCH_LABEL_TEXT}</div>
+        <ToggleSwitch checked={false} onChange={() => handleChange()} aria-labelledby="switchLabel" />
+      </>,
+    )
+    expect(handleChange).not.toHaveBeenCalled()
+
+    // A new `onChange` identity each render previously re-ran the effect and
+    // fired the callback; it must not be called without a user interaction.
+    rerender(
+      <>
+        <div id="switchLabel">{SWITCH_LABEL_TEXT}</div>
+        <ToggleSwitch checked={false} onChange={() => handleChange()} aria-labelledby="switchLabel" />
+      </>,
+    )
+    expect(handleChange).not.toHaveBeenCalled()
+  })
+
   it('can pass data attributes to the rendered component', async () => {
     const TEST_ID = 'a test id'
     const ControlledSwitchComponent = () => {

packages/react/src/ToggleSwitch/ToggleSwitch.tsx

@@ -109,20 +109,21 @@ const ToggleSwitch = React.forwardRef<HTMLButtonElement, ToggleSwitchProps>(func
     e => {
       if (disabled || loading) return
 
-      if (!isControlled) {
+      if (isControlled) {
+        // For controlled usage the click is the source of the change, so notify the
+        // consumer here rather than echoing the `checked` prop back from an effect
+        // (which fired on mount and whenever any of its dependencies — `checked`,
+        // `disabled`, or `onChange` — changed).
+        onChange?.(!isOn)
+      } else {
+        // `setIsOn` notifies `onChange` for uncontrolled usage.
         setIsOn(!isOn)
       }
       onClick && onClick(e)
     },
-    [disabled, isControlled, loading, onClick, setIsOn, isOn],
+    [disabled, isControlled, loading, onClick, onChange, setIsOn, isOn],
   )
 
-  useEffect(() => {
-    if (onChange && isControlled && !disabled) {
-      onChange(Boolean(checked))
-    }
-  }, [onChange, checked, isControlled, disabled])
-
   useEffect(() => {
     if (!loading && isLoadingLabelVisible) {
       // eslint-disable-next-line react-hooks/set-state-in-effect, react-you-might-not-need-an-effect/no-chain-state-updates