fix: detached worklet handlers on Fabric when StrictMode enabled

[kirillzyusko]

📜 Description

On fabric with StrictMode enabled worklet handlers were not working. It was because of the fact that effects are fired twice, and current implementation of useSyncEffect simply added handler and unmounted it when effect is fired twice intentionally.

💡 Motivation and Context

Original PR that introduced these changes was #538

Theoretically this problem could be fixed by using a version like:

import { useEffect, useRef } from "react";

import type { DependencyList } from "react";

/**
 * @description
 * Equivalent to `useEffect` but will run the effect synchronously, i. e. before render.
 *
 * @param {effect} - imperative function
 * @param {deps} - if present, effect will only activate if the values in the list change
 *
 * @author Kiryl Ziusko
 * @since 1.13.0
 * @version 1.0.0
 */
const useSyncEffect: typeof useEffect = (effect, deps) => {
  const cachedDeps = useRef<DependencyList | undefined | null>(null);
  const areDepsEqual = deps?.every(
    (el, index) => cachedDeps.current && el === cachedDeps.current[index],
  );
  const cleanupRef = useRef<(() => void) | void>();

  if (!areDepsEqual || !cachedDeps.current) {
    cleanupRef.current?.();
    cleanupRef.current = effect();
    cachedDeps.current = deps;
  }

  useEffect(() => {
    // strict mode double effect invocation handling
    if (deps !== cachedDeps.current) {
      cleanupRef.current = effect();
      cachedDeps.current = deps;
    }
  }, deps);

  useEffect(() => {
    return () => {
      cleanupRef.current?.();
      cachedDeps.current = null;
    };
  }, []);
};

export default useSyncEffect;

But in this case:

• effects are not running synchronously (second run from react in StrictMode will run it asynchronously anyway because it'll be triggered from useEffect);
• we mutate ref during render which is also not desirable in concurrent react;
• the StrictMode fix can not be covered by unit tests, which indicated that we go too deeply and probably doing something wrong if it can not be covered by unit tests.

So keeping that in my head I decided to make a step back and rewrite this part - now instead of useSyncEffect I will use useLayoutEffect. In this case I'll still run effects slightly earlier than plain useEffect and it should be alright for now. I've tested Expensify app with these changes and now I don't see advantages of using useSyncEffect over useLayoutEffect - both of them give roughly speaking the same result.

I also thought about using useInsertionEffect (I'm not css-in-js library developer, but still) - but in this case I also didn't discover any advantages and react warns that it's better to stick to default useEffect/useLayoutEffect hooks. Potentially we can use useInsertionEffect but it should be a last resort and for now I prefer to use useSyncEffect 😅

Important

Even though we migrated to default lifecycles it still worth to note, that fix #575 is still relevant, as ref may still not be updated by the time when effect is called 🤷‍♂️

📢 Changelog

JS

• remove useSynceEffect hook;
• use useLayoutEffect instead of useEffect hook.

🤔 How Has This Been Tested?

Tested on iPhone 15 Pro, iOS 17.5 (FabricExample app in StrictMode and Expensify app).

📝 Checklist

• CI successfully passed
• I added new mocks and corresponding unit-tests if library API was changed

[github-actions]

📊 Package size report

Current size	Target Size	Difference
148736 bytes	151531 bytes	-2795 bytes 📉