Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
lukaszkrzywizna opened this issue Mar 4, 2024 · 1 comment

Comments

@lukaszkrzywizna
Copy link

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:

  • Fable version: 4.14.0
  • Operating system: MacOS Sonoma
@MangelMaxime
Copy link
Member

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>].

[<Global>]
let setTimeout (callback: unit -> unit) (ms: int) : int = nativeOnly
[<Global>]
let clearTimeout (token: int) : unit = nativeOnly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants