Is there a correct way to override a javascript function in React?

Question

I am detecting popups on a site by overriding window.open with a closure of itself with some additional logic.

const PopupWarning = () => {
  const [failedUrl, setFailedUrl] = useState<string | null>(null)
  const [showPopupWarning, setShowPopupWarning] = useState<boolean>(false)

  window.open = function (open) {
    return function (url, name, features) {
      name = name || 'default_window_name'
      const popup = open.call(window, url, name, features)
      try {
        popup.focus()
        setShowPopupWarning(false)
        setFailedUrl(null)
      } catch (e) {
        setFailedUrl(url.toString())
        setShowPopupWarning(true)
      }
      return popup
    }
  }(window.open)
...

Is this an acceptable way to do something like this? It works during all of my tests, but I wonder if this code should be in a useCallback or other.

I tried searching for this idea and couldn't find much. I bet overriding window.open is frowned upon, but it's the best solution for this use case (can't change source html). Thanks!

I agree that overriding built-in behavior is generally a bad idea. What problem are you trying to solve? You want to alert people to the fact that another window was opened programmatically? And your React app is being rendered into a page you don't otherwise control? — ray
– ray, Commented Nov 2, 2022 at 22:08
Probably the main improvement this needs is that you'll want to do this in useEffect also restore the original window.open when your component unmounts. — Evert
– Evert, Commented Nov 3, 2022 at 6:43
@ray Yeah the app loads predefined HTML that can't be changed and that utilizes window.open in buttons. I need to warn the user if they have pop-ups blocked. — Sean Grant
– Sean Grant, Commented Nov 3, 2022 at 22:02
Given the constraints your solution seems pretty reasonable. Are the buttons that open the popups easily identifiable? Do you have visibility into how they launch the popups? If so you might be able to monkeypatch their event handler or something instead of window.open, but I’m not sure that would necessarily be better than what you have. — ray
– ray, Commented Nov 3, 2022 at 23:43
I feel like the way react would want you to do it is to define a function that implements your open logic then pass that down with a Context Provider. Then the components wouldn't be directly calling window.open at all, just your wrapper function and no monkey patching required but I'm not sure how applicable that is if your pages are mainly static html. — Tadhg McDonald-Jensen
– Tadhg McDonald-Jensen, Commented Nov 9, 2022 at 21:41

Mihályi Zoltán · Accepted Answer · 2022-11-04 14:02:04Z

4

+50

You need to install the window.open proxy on component mount, and uninstall it on unmount. You can use useEffect hook to achieve this.

useEffect(() => {
    const originalOpen = window.open;
    window.open = function (url, name, features) { // install proxy
        name = name || 'default_window_name'
        const popup = open.call(window, url, name, features)
        try {
            popup.focus()
            setShowPopupWarning(false)
            setFailedUrl(null)
        } catch (e) {
            setFailedUrl(url.toString())
            setShowPopupWarning(true)
        }
        return popup;
    };

    return () => {
        window.open = originalOpen; // uninstall proxy
    };
});

answered Nov 4, 2022 at 14:02

Mihályi Zoltán

8826 silver badges13 bronze badges

Sign up to request clarification or add additional context in comments.

Collectives™ on Stack Overflow

Is there a correct way to override a javascript function in React?

1 Answer 1

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

Comments

Your Answer

Sign up or log in

Post as a guest

Related