Production build optimization sometimes break React hooks rules

[lukaszkrzywizna]

Description

When compiling F# code for production, Fable optimizations sometimes seem to break React hooks rules by altering the execution order of hooks. The debug build maintains the correct order and usage of hooks, but the production build optimizes the code in a way that could potentially violate the Rules of Hooks by conditionally executing a hook.

Repro code

The F# source code for the custom hook:

[<Hook>]
let useSmOrAboveVerticalScroll threshold =
  let smOrAbove = React.useScreenSizeOrAbove ScreenSize.Sm
  let scrollRef = useTimesheetScrollRefContext()
  let isThreshold = useVerticalScroll(threshold, scrollRef)
  
  isThreshold && smOrAbove

Debug build JavaScript output:

export function useSmOrAboveVerticalScroll(threshold) {
  const smOrAbove = useReact_useScreenSizeOrAbove_4B6032D5(new ScreenSize(1, []));
  const scrollRef = useTimesheetScrollRefContext();
  const isThreshold = useTransformOnScroll_useVerticalScroll_1DC1393(threshold, scrollRef);
  if (isThreshold) {
      return smOrAbove;
  }
  else {
      return false;
  }
}

Production build JavaScript output (where the issue arises):

export function useSmOrAboveVerticalScroll(threshold) {
  const smOrAbove = useReact_useScreenSizeOrAbove_4B6032D5(new ScreenSize(1, []));
  if (useTransformOnScroll_useVerticalScroll_1DC1393(threshold, useTimesheetScrollRefContext())) {
      return smOrAbove;
  }
  else {
      return false;
  }
}

A workaround involving an extra function call with a hook's dependency as an argument mitigates the issue, but it's not an ideal solution:

[<Hook>]
let useSmOrAboveVerticalScroll threshold =
  let smOrAbove = React.useScreenSizeOrAbove ScreenSize.Sm
  let scrollRef = useTimesheetScrollRefContext()
  let isThreshold = useVerticalScroll(threshold, scrollRef)
  
  ignore scrollRef.current
  
  isThreshold && smOrAbove

export function useSmOrAboveVerticalScroll(threshold) {
    const smOrAbove = useReact_useScreenSizeOrAbove_4B6032D5(new ScreenSize(1, []));
    const scrollRef = useTimesheetScrollRefContext();
    const isThreshold = useTransformOnScroll_useVerticalScroll_1DC1393(threshold, scrollRef);
    scrollRef.current;
    if (isThreshold) {
        return smOrAbove;
    }
    else {
        return false;
    }
}

I wonder if there is any option/attribute that would block that kind of optimization?

Expected and actual results

Expected: The production build should preserve the order and unconditional execution of hooks as per the debug build and React's rules.

Actual: The production build optimizes the code in a way that could conditionally execute a hook, potentially breaking React's rules of hooks and leading to unpredictable component behavior.

Related information

• Fable version: 4.14.0
• Operating system: macOS Sonoma

[MangelMaxime]

I am not familiar with the portion of Fable code which handle these optimisation.

It is possible that the optimisation are done by FCS itself in which case I don't think we can do much unfortunately.

Or this is perhaps done by Fable when applying the transformation:

https://github.com/fable-compiler/Fable/blob/main/src/Fable.Transforms/FableTransforms.fs#L803-L820

The problem is that we need to keep the optimisation for normal JavaScript usage and want to perhaps disable them for React only which Fable is not aware of.

Warning

Below I am just thinking out loud and that's probably not the correct path

If this is coming from Fable perhaps it could be possible to add an attribute or something to disable the optimisation locally and have it applied by [<Hook>] automatically like that any decorated function with [<Hook>] will say please don't optimise me.

[ncave]

@MangelMaxime I agree, it could be either F# compiler (FCS) doing the optimization, or Fable.
If it's in FCS, there may be little we could do, except run in DEBUG mode. If it's in Fable, it tries its best not to beta reduce code that has side effects, so in theory we may be able to improve that detection. Attributes sound like a good second option, if it's in Fable and can't be solved uniformly for all cases.

[lukaszkrzywizna]

Thank you for the information. I guess that [<Hook>] attribute optimisation could be a good idea since it's React that requires the code to be formed in an unusual way.

[ncave]

@lukaszkrzywizna Until we have such an attribute, another workaround that prevents identifiers from being beta-reduced is to mark them as mutable. I know, not very functional, and not very pretty, but at least it doesn't add another line like using ignore. I'm not saying you should use one or the other, It's just another workaround.

let mutable isThreshold = useVerticalScroll(threshold, scrollRef)