Merge branch 'ps-init-recaptcha-script-cleanup-callback' into 'master'

Cleanup callback set in init_recaptcha_script

See merge request gitlab-org/gitlab!52766

app/assets/javascripts/captcha/init_recaptcha_script.js

@@ -2,9 +2,6 @@
 import { memoize } from 'lodash';
 
 export const RECAPTCHA_API_URL_PREFIX = 'https://www.google.com/recaptcha/api.js';
-/**
- * The name which will be used for the reCAPTCHA script's onload callback
- */
 export const RECAPTCHA_ONLOAD_CALLBACK_NAME = 'recaptchaOnloadCallback';
 
 /**
@@ -21,9 +18,7 @@ export const RECAPTCHA_ONLOAD_CALLBACK_NAME = 'recaptchaOnloadCallback';
  *
  */
 export const initRecaptchaScript = memoize(() => {
-  /**
-   * Appends the the reCAPTCHA script tag to the head of document
-   */
+  // Appends the the reCAPTCHA script tag to the head of document
   const appendRecaptchaScript = () => {
     const script = document.createElement('script');
     script.src = `${RECAPTCHA_API_URL_PREFIX}?onload=${RECAPTCHA_ONLOAD_CALLBACK_NAME}&render=explicit`;
@@ -31,11 +26,14 @@ export const initRecaptchaScript = memoize(() => {
     document.head.appendChild(script);
   };
 
-  /**
-   * Returns a Promise which is fulfilled after the reCAPTCHA script is loaded
-   */
   return new Promise((resolve) => {
-    window[RECAPTCHA_ONLOAD_CALLBACK_NAME] = resolve;
+    // This global callback resolves the Promise and is passed by name to the reCAPTCHA script.
+    window[RECAPTCHA_ONLOAD_CALLBACK_NAME] = (val) => {
+      // Let's clean up after ourselves. This is also important for testing, because `window` is NOT cleared between tests.
+      // https://github.com/facebook/jest/issues/1224#issuecomment-444586798.
+      delete window[RECAPTCHA_ONLOAD_CALLBACK_NAME];
+      resolve(val);
+    };
     appendRecaptchaScript();
   });
 });

spec/frontend/captcha/init_recaptcha_script_spec.js

@@ -7,19 +7,11 @@ import {
 
 describe('initRecaptchaScript', () => {
   afterEach(() => {
-    // NOTE: The DOM is guaranteed to be clean at the start of a new test file, but it isn't cleaned
-    // between examples within a file, so we need to clean it after each one.  See more context here:
-    // - https://github.com/facebook/jest/issues/1224
-    // - https://stackoverflow.com/questions/42805128/does-jest-reset-the-jsdom-document-after-every-suite-or-test
-    //
-    // Also note as mentioned in https://github.com/facebook/jest/issues/1224#issuecomment-444586798
-    // that properties of `window` are NOT cleared between test files. So, we are also
-    // explicitly unsetting it.
     document.head.innerHTML = '';
-    window[RECAPTCHA_ONLOAD_CALLBACK_NAME] = undefined;
     clearMemoizeCache();
   });
 
+  const getScriptOnload = () => window[RECAPTCHA_ONLOAD_CALLBACK_NAME];
   const triggerScriptOnload = (...args) => window[RECAPTCHA_ONLOAD_CALLBACK_NAME](...args);
 
   describe('when called', () => {
@@ -51,6 +43,7 @@ describe('initRecaptchaScript', () => {
       triggerScriptOnload(instance);
 
       await expect(result).resolves.toBe(instance);
+      expect(getScriptOnload()).toBeUndefined();
     });
   });
 });