Simplify FeatureFlags ref counting by removing WeakMap

[Copilot]

Simplifies the data-dialog-scroll-optimized attribute ref counting in FeatureFlags by removing unnecessary WeakMap-based instance tracking. The WeakMap was guarding against double-counting, but React's useEffect cleanup semantics already prevent this.

Changes

Removed:

• WeakMap instance tracking (dialogScrollOptimizedInstances)
• useRef and instanceRef usage
• Conditional checks in useEffect

Added:

• __resetDialogScrollOptimizedCount() function for test isolation (not exported from public API)
• JSDoc explaining this is temporary infrastructure during feature flag rollout

Simplified:

// Before: 15+ lines with WeakMap checks
const instance = instanceRef.current
if (!dialogScrollOptimizedInstances.get(instance)) {
  dialogScrollOptimizedInstances.set(instance, true)
  dialogScrollOptimizedCount++
  // ...
}

// After: Direct counter manipulation
dialogScrollOptimizedCount++
document.body.setAttribute('data-dialog-scroll-optimized', '')

Tests import reset function directly from FeatureFlags.tsx (bypassing public index) for proper isolation.

Changelog

Changed

• Simplified FeatureFlags ref counting to use counter-only approach instead of WeakMap + counter

Rollout strategy

• Patch release
• Minor release
• Major release; if selected, include a written rollout or migration plan
• None; if selected, include a brief description as to why

Testing & Reviewing

All existing tests pass including multi-instance ref counting scenarios. The __resetDialogScrollOptimizedCount function ensures test isolation without being exposed in the public API.

Merge checklist

• Added/updated tests
• Added/updated documentation
• Added/updated previews (Storybook)
• Changes are SSR compatible
• Tested in Chrome
• Tested in Firefox
• Tested in Safari
• Tested in Edge
• (GitHub staff only) Integration tests pass at github/github (Learn more about how to run integration tests)

Original prompt

Summary

Simplify the ref counting implementation in PR #7393 by removing the WeakMap and using a simple counter instead. Also expose a test-only reset function that is not part of the public API.

Background

The current implementation uses both a WeakMap and a counter for ref counting, which is overengineered:

const dialogScrollOptimizedInstances = new WeakMap<object, boolean>()
let dialogScrollOptimizedCount = 0

This can be simplified to just a counter since:

• The WeakMap only guards against double-counting, but useEffect cleanup semantics already prevent that
• The count is doing the real work anyway
• WeakMap adds complexity without real benefit

Changes Needed

1. Simplify packages/react/src/FeatureFlags/FeatureFlags.tsx

Replace the WeakMap + count approach with a simple counter:

import type React from 'react'
import {useContext, useMemo, useEffect} from 'react'
import {FeatureFlagContext} from './FeatureFlagContext'
import {FeatureFlagScope, type FeatureFlags} from './FeatureFlagScope'

export type FeatureFlagsProps = React.PropsWithChildren<{
  flags: FeatureFlags
}>

/**
 * Ref count for data-dialog-scroll-optimized attribute management.
 * 
 * NOTE: This is temporary infrastructure while we feature flag the CSS :has() 
 * performance optimization (primer_react_css_has_selector_perf). Once the flag
 * is removed and the optimization is the default behavior, this ref counting
 * can be removed - the attribute can simply always be present.
 * 
 * @internal - Not part of the public API
 */
let dialogScrollOptimizedCount = 0

/**
 * Reset the ref count for testing purposes only.
 * 
 * @internal - Not part of the public API. Only exported for test isolation.
 */
export function __resetDialogScrollOptimizedCount(): void {
  dialogScrollOptimizedCount = 0
  document.body.removeAttribute('data-dialog-scroll-optimized')
}

export function FeatureFlags({children, flags}: FeatureFlagsProps) {
  const parentFeatureFlags = useContext(FeatureFlagContext)
  const value = useMemo(() => {
    const scope = FeatureFlagScope.merge(parentFeatureFlags, FeatureFlagScope.create(flags))
    return scope
  }, [parentFeatureFlags, flags])

  const isOptimizationEnabled = value.enabled('primer_react_css_has_selector_perf')

  // Set body attribute for CSS :has() optimization when flag is enabled
  useEffect(() => {
    if (isOptimizationEnabled) {
      dialogScrollOptimizedCount++
      document.body.setAttribute('data-dialog-scroll-optimized', '')
      
      return () => {
        dialogScrollOptimizedCount--
        if (dialogScrollOptimizedCount === 0) {
          document.body.removeAttribute('data-dialog-scroll-optimized')
        }
      }
    }
  }, [isOptimizationEnabled])

  return <FeatureFlagContext.Provider value={value}>{children}</FeatureFlagContext.Provider>
}

2. Update the test file packages/react/src/FeatureFlags/__tests__/FeatureFlags.test.tsx

Use the reset function in beforeEach instead of manual attribute cleanup:

import {describe, expect, it, beforeEach} from 'vitest'
import {render} from '@testing-library/react'
import {FeatureFlags, useFeatureFlag, __resetDialogScrollOptimizedCount} from '../../FeatureFlags'

describe('FeatureFlags', () => {
  beforeEach(() => {
    // Reset module state between tests for isolation
    __resetDialogScrollOptimizedCount()
  })

  // ... rest of tests unchanged
})

3. Ensure the reset function is NOT exported from the public index

Check packages/react/src/FeatureFlags/index.ts (or wherever the public exports are) and ensure __resetDialogScrollOptimizedCount is NOT re-exported. It should only be importable directly from the FeatureFlags.tsx file for tests.

The double-underscore prefix (__) is a common convention indicating internal/private APIs.

4. Remove the instanceRef

The useRef({}) is no longer needed since we're not using WeakMap:

- const instanceRef = useRef({})

Documentation

Add a comment in the code explaining this is temporary:

/**
 * Ref count for data-dialog-scroll-optimized attribute management.
 * 
 * NOTE: This is temporary infrastructure while we feature flag the CSS :has() 
 * performance optimization (primer_react_css_has_selector_perf). Once the flag
 * is removed and the optimization is the default behavior, this ref counting
 * can be removed - the attribute can simply always be present.
 * 
 * Target removal: Q1 2025 (after flag rollout complete)
 * 
 * @internal - Not part of the public API
 */

Testing

• Verify all existing tests still pass
• Verify the reset function properly isolates tests
• Verify the reset function is not accessible from the public API (check index exports)

This pull request was created from Copilot chat.

---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 4310e58

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[github-actions]

👋 Hi, this pull request contains changes to the source code that github/github-ui depends on. If you are GitHub staff, test these changes with github/github-ui using the integration workflow. Or, apply the integration-tests: skipped manually label to skip these checks.