fable-compiler / Fable

F# to JavaScript, TypeScript, Python, Rust and Dart Compiler
http://fable.io/
MIT License
2.93k stars 301 forks source link

Local Function Name Clashes with Global JavaScript Functions Leading to Incorrect Behavior #3775

Open lukaszkrzywizna opened 9 months ago

lukaszkrzywizna commented 9 months ago

Description:

When defining a local F# function in a Fable project with a name that matches a global JavaScript function (e.g., setTimeout, clearTimeout), Fable always references the local function, even in contexts where the global JavaScript function is intended to be used. This behavior can lead to unexpected errors, such as "RangeError: Maximum call stack size exceeded", due to recursive calls when the local function tries to use the global one indirectly.

Repro code:

module Client.Test

open Feliz

[<ReactComponent>]
let MyComponent () =
  let timeoutRef = React.useRef None

  // This local function unintentionally shadows the global JS clearTimeout function
  let clearTimeout () =
    timeoutRef.current
    |> Option.iter Fable.Core.JS.clearTimeout // Intended to use global clearTimeout

  let startTimeout () =
    timeoutRef.current <- Some (Fable.Core.JS.setTimeout (fun () -> printfn "Hello") 3000)

  Html.div [
    Html.button [
      prop.className "px-3 py-2 border"
      prop.onClick (fun _ -> startTimeout ())
      prop.text "Start timeout"
    ]
    Html.button [
      prop.className "px-3 py-2 border"
      prop.onClick (fun _ -> clearTimeout ())
      prop.text "Clear timeout"
    ]
  ]

Expected and actual results:

Expected: The clearTimeout call within the local clearTimeout function should reference the global JavaScript clearTimeout function, allowing for the proper clearing of timeouts set with setTimeout.

Actual: The local clearTimeout function is recursively calling itself instead of the global JavaScript clearTimeout, leading to a "RangeError: Maximum call stack size exceeded" error.

import { createElement } from "react";
import React from "react";
import { useReact_useRef_1505 } from "./fable_modules/Feliz.2.7.0/React.fs.js";
import { iterate } from "./fable_modules/fable-library-js.4.14.0/Seq.js";
import { toArray } from "./fable_modules/fable-library-js.4.14.0/Option.js";
import { printf, toConsole } from "./fable_modules/fable-library-js.4.14.0/String.js";
import { ofArray } from "./fable_modules/fable-library-js.4.14.0/List.js";
import { Interop_reactApi } from "./fable_modules/Feliz.2.7.0/Interop.fs.js";

export function MyComponent() {
    const timeoutRef = useReact_useRef_1505(void 0);
    const clearTimeout = () => {
        iterate((token) => {
            clearTimeout(token);
        }, toArray(timeoutRef.current));
    };
    const startTimeout = () => {
        timeoutRef.current = setTimeout(() => {
            toConsole(printf("Hello"));
        }, 3000);
    };
    const children = ofArray([createElement("button", {
        className: "px-3 py-2 border",
        onClick: (_arg) => {
            startTimeout();
        },
        children: "Start timeout",
    }), createElement("button", {
        className: "px-3 py-2 border",
        onClick: (_arg_1) => {
            clearTimeout();
        },
        children: "Clear timeout",
    })]);
    return createElement("div", {
        children: Interop_reactApi.Children.toArray(Array.from(children)),
    });
}

//# sourceMappingURL=Test.js.map

Suggested Solution:

It might be beneficial for Fable to provide clearer warnings or errors during compilation when local function names shadow global JavaScript functions, or potentially offer a way to explicitly reference global functions in such cases.

Related information:

MangelMaxime commented 9 months ago

A workaround is to use window.clearTimeout you can use:

open Browser

open Browser

let clearTimeout () =
    None
    |> Option.iter window.clearTimeout

It will generates:

import { iterate } from "fable-library-js/Seq.js";
import { toArray } from "fable-library-js/Option.js";

export function clearTimeout() {
    iterate((handle) => {
        window.clearTimeout(handle);
    }, toArray(void 0));
}

You could also create a local alias to the function:

open Browser

let jsClearTimeout = Fable.Core.JS.clearTimeout
let inline jsClearTimeoutInlined delay = Fable.Core.JS.clearTimeout delay

let clearTimeout () =
    None
    |> Option.iter jsClearTimeout

let clearTimeout2 () =
    None
    |> Option.iter jsClearTimeoutInlined

I think current Fable mechanism for detecting name clashing don't catch this situation because clearTimeout is decorated with [<Global>].

https://github.com/fable-compiler/Fable/blob/c287b1502875bc749687df3e382d28123a676ea6/src/Fable.Core/Fable.Core.JS.fs#L771-L775