React rerender causing multiple captcha solves

[MauriceArikoglu]

Following the Example in the Documentation#react-example, we are seeing issues with auto-solving captchas following re-renders from react.

Calling destroy() instead of reset() seems to fix the problem, but I feel like either the js implementation in the friendly-challenge lib is somewhat irregular or the documentation/example for react is incorrect. I also say "seems", because I fear side effects caused by calling functions not clearly defining their side effects. (What is destroy anyway, and why do we need it explicitly (compared to a captcha in a "solved" state)?)

I think the re-render issue stems from this line in the reset() function, especially in connection with startMode="auto", as it immediately restarts the solving process: https://github.com/FriendlyCaptcha/friendly-challenge/blob/87282feb6f494a9dfd1fd2378f9abc097411c740/src/captcha.ts#L294

[gzuidhof]

Thank you for reporting this, I think that #72 might be the same issue, this adds more of a targeted direction to help solve it. I think you're right that the widget or React lib doesn't work correctly in this case - what I expect is happening is that during a solve the solution of the previous puzzle which may have been in progress is still somehow gets used. That may be (partially) solved by #75.

@Fancy11111 would you maybe have time to investigate this? I believe you already made a start somewhere.

[MauriceArikoglu]

@gzuidhof I am about to finish refactoring our implementation. If you want, I can publish the component code to our github and you can have a look at it. If it's working for you, feel free to update your documentation and link to that instead...

We are also using NextJS in some projects and have an adapter for NextJS as well, providing next's client side dynamic import. I could also provide that but would previously need to trim it.

[gzuidhof]

That would be really helpful, thank you! There is a Next.js example here (it needs a final bit of polish, but it's functional): https://github.com/FriendlyCaptcha/friendly-captcha-examples/tree/main/nextjs.

I haven't tested whether it suffers from the same issue mentioned in the original post. My intuition tells me the solution for that lies in the widget itself not in the React/NextJS integration (although it could of course work around it).

[MauriceArikoglu]

@gzuidhof pasted below our React implementation for FriendlyCaptcha:

import React, { useEffect, useRef } from "react";
import { WidgetInstance } from "friendly-challenge";

// Improvement: Use ENV var
// Replace: with non-EU Endpoint if required
const CAPTCHA_URL = 'https://eu-api.friendlycaptcha.eu/api/v1/puzzle';

type IFriendlyCaptchaProps = {
  siteKey: string;
  doneCallback: (solution: string) => void;
  errorCallback: (error: any) => void;
};

const FriendlyCaptcha: React.FC<
  IFriendlyCaptchaProps & React.HTMLAttributes<HTMLDivElement>
> = (props) => {
  const container = useRef<HTMLDivElement | null>(null);
  const widget = useRef<WidgetInstance | null>(null);

  useEffect(() => {
    if (!widget.current && container.current) {
      widget.current = new WidgetInstance(container.current, {
        puzzleEndpoint: CAPTCHA_URL,
// Replace startMode as needed
        startMode: 'auto',
        doneCallback: props.doneCallback,
        errorCallback: props.errorCallback,
      });
    }

    return () => {
      if (widget.current != undefined) {
        widget.current.destroy();
      }
    };
  }, [props]);

  return (
    <div
      ref={container}
      // Add the frc-captcha class if you want to use FriendlyCaptcha styling
      className={props.className}
      data-sitekey={props.siteKey}
    />
  );
};

export default React.memo(FriendlyCaptcha);

[MauriceArikoglu]

For NextJS I can't publish our full implementation sadly, as there's other strings attached there. Also can't publish a clean implementation without our custom stuff. Most important things are:

• Use next's dynamic imports without SSR
• have one clear state in which the captcha is acting and nothing else. -> Don't render it every single time

Next dynamic:

// DC = DynamicComponent
const DCFriendlyCaptcha = dynamic(
  () => import('@components/FriendlyCaptcha'),
  { ssr: false }
);

[gzuidhof]

For NextJS I can't publish our full implementation sadly, as there's other strings attached there. Also can't publish a clean implementation without our custom stuff.

That's completely understandable! Thank you for your implementation and all the info you've shared so far - it's super useful.

---

I just received a support query that ended up in invalid solutions, with a snippet similar to this

const options = {
  doneCallback: someCallback,
  sitekey: "<some sitekey>",
  startMode: "auto",
};

const widget = new WidgetInstance(element, options);
widget.start();

The combination of widget.start() as well as startMode: "auto" was problematic. I think startMode: "auto" is at the root of the issues here like you mentioned and requires a fix in the widget.

@Fancy11111 ping to be aware of this :)

[merlinfuchs]

I think using .destroy() instead of .reset() in the cleanup function is actually the correct thing to do. We are not re-using the same Widget instance so using .reset() will start solving a puzzle in the old Widget instance which isn't attached to the DOM anymore. This can also lead to doneCallback being called multiple times. (once from the old instance and once from the new instance)

I'm pretty sure this isn't a bug in the widget implementation, the react example is just using the wrong function to do the job. (vue example, and nextjs examples as well btw) We should either re-use the same widget instance or replace reset with destroy in the examples. There isn't really a benefit in re-using the same widget instance atm (the workers are terminated anway) so I would suggest to just use .destroy().

[MauriceArikoglu]

Thank you @merlinfuchs