vscode-webview-ui-toolkit icon indicating copy to clipboard operation
vscode-webview-ui-toolkit copied to clipboard

VSCodeCheckbox onChange handler called on first render

Open deerob4 opened this issue 3 years ago • 2 comments

Describe the bug

VSCodeCheckbox React components that have a checked value which references a React state variable initialised to true will call their onChange prop on first render. Since the onChange prop is usually toggling that same state variable, the component enters into an infinite recursive loop.

This actually seems to happen when useState is initialised with anything other than false. I tried a bunch of other values, truthy and falsy, and they all triggered the bug. It doesn't occur when the checked prop has a literal value of true.

To reproduce

The problem can be reproduced with the following code:

import React, { FunctionComponent, useState } from "react";
import { VSCodeCheckbox } from "@vscode/webview-ui-toolkit/react";

const MyPage: FunctionComponent = () => {
  const [isChecked, setIsChecked] = useState(true);

  function toggleChecked() {
    console.log("I'll be called on first render");
    setIsChecked(checked => !checked)
  }

  return (
    <VSCodeCheckbox onChange={toggleChecked} checked={isChecked}>
      Toggle checked
    </VSCodeCheckbox>
  );
};

Expected behavior

The onChange handler should only be called when the checkbox is first clicked, like normal <input type="checkbox" /> elements.

Current behavior

The onChange handler is called immediately on first render, which results in an infinite loop if it toggles the value read by checked.

Desktop (please complete the following information):

  • OS Version: macOS 12.6
  • Toolkit Version: v1.1.0

deerob4 avatar Sep 22 '22 18:09 deerob4

I've also encountered such an issue, the component keeps re-rendering util its max-depth when triggered onChanged event. So I had to write the radio buttons using raw HTML.

kevin-s-wang avatar Sep 23 '22 06:09 kevin-s-wang

Thanks again for the bug report @deerob4!

Like I said in your other issue, I've been a bit busy with other work but will also put this on the top of my todo list once I have some free cycles 😊

hawkticehurst avatar Sep 26 '22 19:09 hawkticehurst