fix: Support React 19 ref cleanup functions in ScrollView

[ScypyonX]

Summary:

ScrollView's createRefForwarder function did not support React 19's new ref system where ref callbacks can return cleanup functions. This caused memory leaks on the C++ side as ShadowNodes couldn't be properly unmounted when ScrollView components were destroyed.

The fix modifies createRefForwarder to detect when ref callbacks return cleanup functions, store them, and call them appropriately when refs change or components unmount, while maintaining backward compatibility with legacy ref behavior.

Resolves #51878

Changelog:

[GENERAL] [FIXED] - ScrollView now properly supports React 19 ref cleanup functions, preventing memory leaks

Test Plan:

1. Created test app using React 19 ref callbacks that return cleanup functions
2. Before fix: Cleanup functions were never called, refs received null on unmount (legacy behavior)
3. After fix: Cleanup functions are properly invoked when ScrollView unmounts
4. Verified backward compatibility - existing ref usage continues to work
5. Tested on iOS simulator - no regressions observed

Test code used:

const scrollViewRef = (instance) => {
  if (instance) {
    console.log('ScrollView mounted');
    return () => {
      console.log('ScrollView cleanup called!'); // This now works with the fix
    };
  }
};

<ScrollView ref={scrollViewRef} />

Result: Cleanup functions are now properly called, resolving the memory leak issue reported in #51878.

// App.tsx - Fixed ScrollView Ref Cleanup Test
import React, { useState, useCallback } from 'react';
import {
  SafeAreaView,
  ScrollView,
  Text,
  View,
  Button,
  StyleSheet,
  Alert,
} from 'react-native';

interface LogEntry {
  id: number;
  message: string;
  timestamp: string;
}

const TestScrollViewRefCleanup: React.FC = () => {
  const [showScrollView, setShowScrollView] = useState<boolean>(true);
  const [logs, setLogs] = useState<LogEntry[]>([]);
  const [mountCount, setMountCount] = useState<number>(0);
  const [cleanupCount, setCleanupCount] = useState<number>(0);
  const [logId, setLogId] = useState<number>(0);

  const addLog = useCallback((message: string) => {
    const timestamp = new Date().toLocaleTimeString();
    console.log(`[${timestamp}] ${message}`);

    setLogs(prev => [
      ...prev,
      {
        id: Date.now() + Math.random(),
        message,
        timestamp,
      },
    ]);
  }, []);

  // React 19 ref callback that returns cleanup function
  const scrollViewRef = useCallback((instance: ScrollView | null) => {
    if (instance) {
      setMountCount(prev => {
        const newCount = prev + 1;
        const message = `📱 ScrollView mounted (count: ${newCount})`;
        const timestamp = new Date().toLocaleTimeString();
        console.log(`[${timestamp}] ${message}`);

        setLogs(prevLogs => [
          ...prevLogs,
          {
            id: Date.now() + Math.random(),
            message,
            timestamp,
          },
        ]);

        return newCount;
      });

      // Return cleanup function (React 19 feature)
      return () => {
        setCleanupCount(prev => {
          const newCount = prev + 1;
          const message = `🧹 ScrollView cleanup called! (count: ${newCount})`;
          const timestamp = new Date().toLocaleTimeString();
          console.log(`[${timestamp}] ${message}`);

          setLogs(prevLogs => [
            ...prevLogs,
            {
              id: Date.now() + Math.random(),
              message,
              timestamp,
            },
          ]);

          return newCount;
        });
      };
    } else {
      // Legacy behavior - should NOT happen with our fix
      const message = '⚠️ Legacy ref callback with null (BAD!)';
      const timestamp = new Date().toLocaleTimeString();
      console.log(`[${timestamp}] ${message}`);

      setLogs(prevLogs => [
        ...prevLogs,
        {
          id: Date.now() + Math.random(),
          message,
          timestamp,
        },
      ]);
    }
  }, []);

  const toggleScrollView = useCallback(() => {
    const message = `🔄 Toggling ScrollView (will ${
      showScrollView ? 'hide' : 'show'
    })`;
    addLog(message);
    setShowScrollView(!showScrollView);
  }, [showScrollView, addLog]);

  const clearLogs = useCallback(() => {
    setLogs([]);
    setMountCount(0);
    setCleanupCount(0);
    setLogId(0);
  }, []);

  const showResults = useCallback(() => {
    const isWorking = cleanupCount > 0;
    const message = isWorking
      ? `✅ SUCCESS!\n\nMounts: ${mountCount}\nCleanups: ${cleanupCount}\n\nReact 19 ref cleanup is working!`
      : `❌ FAILED!\n\nMounts: ${mountCount}\nCleanups: ${cleanupCount}\n\nReact 19 ref cleanup is NOT working.\nScrollView is using legacy behavior.`;

    Alert.alert('Test Results', message);
  }, [mountCount, cleanupCount]);

  return (
    <SafeAreaView style={styles.container}>
      <View style={styles.header}>
        <Text style={styles.title}>ScrollView Ref Cleanup Test</Text>
        <Text style={styles.subtitle}>
          Testing React 19 ref cleanup functions
        </Text>
      </View>

      <View style={styles.controls}>
        <Button title="Toggle ScrollView" onPress={toggleScrollView} />
        <View style={styles.buttonSpacing} />
        <Button title="Show Results" onPress={showResults} color="#007AFF" />
        <View style={styles.buttonSpacing} />
        <Button title="Clear Logs" onPress={clearLogs} color="#FF3B30" />
      </View>

      <View style={styles.stats}>
        <Text style={styles.statText}>Mounts: {mountCount}</Text>
        <Text style={styles.statText}>Cleanups: {cleanupCount}</Text>
        <Text
          style={[
            styles.statText,
            { color: cleanupCount > 0 ? 'green' : 'red' },
          ]}
        >
          Status: {cleanupCount > 0 ? 'WORKING ✅' : 'NOT WORKING ❌'}
        </Text>
      </View>

      {showScrollView && (
        <ScrollView
          ref={scrollViewRef}
          style={styles.scrollView}
          contentContainerStyle={styles.scrollContent}
        >
          <Text style={styles.scrollText}>
            🧪 This is a test ScrollView with React 19 ref cleanup
          </Text>
          <Text style={styles.instruction}>
            Instructions:{'\n'}
            1. Click "Toggle ScrollView" to hide this ScrollView{'\n'}
            2. Check if cleanup function was called in logs{'\n'}
            3. Click "Show Results" to see if the fix works{'\n'}
            4. Toggle multiple times to test repeated mount/unmount
          </Text>
          {Array.from({ length: 20 }, (_, i) => (
            <Text key={i} style={styles.listItem}>
              List item {i + 1} - Scroll me!
            </Text>
          ))}
        </ScrollView>
      )}

      <View style={styles.logsContainer}>
        <Text style={styles.logsTitle}>Logs:</Text>
        <ScrollView style={styles.logsScroll}>
          {logs.map(log => (
            <Text key={log.id} style={styles.logItem}>
              {log.timestamp}: {log.message}
            </Text>
          ))}
        </ScrollView>
      </View>
    </SafeAreaView>
  );
};

const styles = StyleSheet.create({
  container: {
    flex: 1,
    backgroundColor: '#f5f5f5',
  },
  header: {
    padding: 20,
    backgroundColor: '#fff',
    borderBottomWidth: 1,
    borderBottomColor: '#ddd',
  },
  title: {
    fontSize: 20,
    fontWeight: 'bold',
    textAlign: 'center',
  },
  subtitle: {
    fontSize: 14,
    color: '#666',
    textAlign: 'center',
    marginTop: 5,
  },
  controls: {
    flexDirection: 'row',
    justifyContent: 'space-around',
    padding: 15,
    backgroundColor: '#fff',
    marginTop: 10,
  },
  buttonSpacing: {
    width: 10,
  },
  stats: {
    flexDirection: 'row',
    justifyContent: 'space-around',
    padding: 15,
    backgroundColor: '#fff',
    marginTop: 10,
  },
  statText: {
    fontSize: 14,
    fontWeight: 'bold',
  },
  scrollView: {
    flex: 1,
    margin: 10,
    backgroundColor: '#fff',
    borderRadius: 8,
  },
  scrollContent: {
    padding: 20,
  },
  scrollText: {
    fontSize: 16,
    fontWeight: 'bold',
    marginBottom: 15,
    textAlign: 'center',
  },
  instruction: {
    fontSize: 14,
    lineHeight: 20,
    marginBottom: 20,
    backgroundColor: '#f0f8ff',
    padding: 15,
    borderRadius: 8,
  },
  listItem: {
    fontSize: 16,
    paddingVertical: 10,
    borderBottomWidth: 1,
    borderBottomColor: '#eee',
  },
  logsContainer: {
    height: 200,
    margin: 10,
    backgroundColor: '#fff',
    borderRadius: 8,
    padding: 10,
  },
  logsTitle: {
    fontSize: 16,
    fontWeight: 'bold',
    marginBottom: 10,
  },
  logsScroll: {
    flex: 1,
  },
  logItem: {
    fontSize: 12,
    paddingVertical: 2,
    fontFamily: 'Courier New',
    color: '#333',
  },
});

export default TestScrollViewRefCleanup;

[facebook-github-bot]

Hi @WojciechMichalowskii!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

[react-native-bot]

	Warnings
⚠️	❗ JavaScript API change detected - This PR commits an update to ReactNativeApi.d.ts, indicating a change to React Native's public JavaScript API. Please include a clear changelog message. This change will be subject to extra review. This change was flagged as: BREAKING

Generated by 🚫 dangerJS against d05596c

[facebook-github-bot]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

[yungsters]

Thanks for the contribution!

I think it would be more forward-looking and easier to read if you instead change the returned getForwardingRef to return a callback function that always returns a cleanup function.

[ScypyonX]

@yungsters Thanks for answer !

The Problem:
React does not automatically call cleanup functions returned from ref callbacks. This is a fundamental difference between ref callbacks and useEffect:

useEffect: useEffect(() => { return cleanup; }, []) - React automatically calls the returned cleanup function
Ref callbacks: ref={(instance) => { return cleanup; }} - React ignores the returned value entirely

What happens with the suggested change:

Component mounts → ref callback called with instance → cleanup function returned (but ignored by React)
Component unmounts → ref callback called with null → new cleanup function returned (but previous cleanup never called)
Result: Previous instance never gets cleaned up, tests fail expecting null

Tested code:

type RefForwarder<TNativeInstance, TPublicInstance> = {
  getForwardingRef: (
    ?React.RefSetter<TPublicInstance>,
  ) => (TNativeInstance | null) => () => void,
  nativeInstance: TNativeInstance | null,
  publicInstance: TPublicInstance | null,
};

/**
 * Helper function that should be replaced with `useCallback` and `useMergeRefs`
 * once `ScrollView` is reimplemented as a functional component.
 */
function createRefForwarder<TNativeInstance, TPublicInstance>(
  mutator: TNativeInstance => TPublicInstance,
): RefForwarder<TNativeInstance, TPublicInstance> {
  const state: RefForwarder<TNativeInstance, TPublicInstance> = {
    getForwardingRef: memoize(forwardedRef => {
      let cleanup: ?() => void = null;

      return (nativeInstance: TNativeInstance | null): (() => void) => {
        if (cleanup) {
          cleanup();
          cleanup = null;
        }

        const publicInstance =
          nativeInstance == null ? null : mutator(nativeInstance);

        state.nativeInstance = nativeInstance;
        state.publicInstance = publicInstance;

        if (forwardedRef != null) {
          if (typeof forwardedRef === 'function') {
            const result = forwardedRef(publicInstance);
            if (typeof result === 'function') {
              // $FlowFixMe[incompatible-use]
              cleanup = result;
            }
          } else {
            forwardedRef.current = publicInstance;
          }
        }

        return () => {
          if (cleanup) {
            cleanup();
            cleanup = null;
          }
        };
      };
    }),
    nativeInstance: null,
    publicInstance: null,
  };

  return state;
}

[yungsters]

React 19 brought support for ref cleanup functions:

https://react.dev/blog/2024/12/05/react-19#cleanup-functions-for-refs

[ScypyonX]

You are right - thanks for update my knowledge :) Fix pushed.

[ScypyonX]

@yungsters any updates?

[ScypyonX]

@yungsters any updates?

[react-native-bot]

This PR is stale because it has been open for 180 days with no activity. It will be closed in 7 days unless you comment on it or remove the "Stale" label.

[react-native-bot]

This PR was closed because it has been stalled for 7 days with no activity.