fix: Refactor initialisation of the formBuilder plugin to ensure that…

… two or more concurrent initialisations cannot interfere with each other
kevinchappell · Oct 19, 2023 · 94894a5 · 94894a5
1 parent b34c8bf
commit 94894a5
Show file tree

Hide file tree

Showing 2 changed files with 102 additions and 54 deletions.
diff --git a/src/js/form-builder.js b/src/js/form-builder.js
@@ -2489,68 +2489,70 @@ function FormBuilder(opts, element, $) {
   return formBuilder
 }
 
-const methods = {
-  init: (options, elems) => {
-    const { i18n, ...opts } = jQuery.extend({}, defaultOptions, options, true)
-    config.opts = opts
-    const i18nOpts = jQuery.extend({}, defaultI18n, i18n, true)
-    methods.instance = {
-      actions: {
-        getFieldTypes: null,
-        addField: null,
-        clearFields: null,
-        closeAllFieldEdit: null,
-        getData: null,
-        removeField: null,
-        save: null,
-        setData: null,
-        setLang: null,
-        showData: null,
-        showDialog: null,
-        toggleAllFieldEdit: null,
-        toggleFieldEdit: null,
-        getCurrentFieldId: null,
-      },
-      markup,
-      get formData() {
-        return methods.instance.actions.getData && methods.instance.actions.getData('json')
-      },
-      promise: new Promise(function (resolve, reject) {
-        mi18n
-          .init(i18nOpts)
-          .then(() => {
-            elems.each(i => {
-              const formBuilder = new FormBuilder(opts, elems[i], jQuery)
-              jQuery(elems[i]).data('formBuilder', formBuilder)
-              Object.assign(methods, formBuilder.actions, { markup })
-              methods.instance.actions = formBuilder.actions
-            })
-            delete methods.instance.promise
-            resolve(methods.instance)
-          })
-          .catch(err => {
-            reject(err)
-            opts.notify.error(err)
-          })
-      }),
-    }
+const pluginInit = function(options,elem) {
+  const _this = this
+  const { i18n, ...opts } = jQuery.extend({}, defaultOptions, options, true)
+  config.opts = opts
+  this.i18nOpts = jQuery.extend({}, defaultI18n, i18n, true)
+
+  const notInitialised = () => {
+    console.error('formBuilder is still initialising')
+    console.info('See https://formbuilder.online/docs/formBuilder/actions/getData/#wont-work and https://formbuilder.online/docs/formBuilder/promise/ for more information on formBuilder asynchronous loading')
+  }
 
-    return methods.instance
-  },
+  this.instance = {
+    actions: {
+      getFieldTypes: notInitialised,
+      addField: notInitialised,
+      clearFields: notInitialised,
+      closeAllFieldEdit: notInitialised,
+      getData: notInitialised,
+      removeField: notInitialised,
+      save: notInitialised,
+      setData: notInitialised,
+      setLang: notInitialised,
+      showData: notInitialised,
+      showDialog: notInitialised,
+      toggleAllFieldEdit: notInitialised,
+      toggleFieldEdit: notInitialised,
+      getCurrentFieldId: notInitialised,
+    },
+    markup,
+    get formData() {
+      return _this.instance.actions.getData !== notInitialised && _this.instance.actions.getData('json')
+    },
+    promise: new Promise(function(resolve, reject) {
+      mi18n
+        .init(_this.i18nOpts)
+        .then(() => {
+          const formBuilder = new FormBuilder(opts, elem[0], jQuery)
+          jQuery(elem[0]).data('formBuilder', formBuilder)
+          Object.assign(_this.instance, formBuilder.actions)
+          _this.instance.actions = formBuilder.actions
+          delete _this.instance.promise
+          resolve(_this.instance)
+        })
+        .catch(err => {
+          reject(err)
+          opts.notify.error(err)
+        })
+    })
+  }
 }
 
 jQuery.fn.formBuilder = function (methodOrOptions = {}, ...args) {
   const isMethod = typeof methodOrOptions === 'string'
   if (isMethod) {
-    if (methods[methodOrOptions]) {
-      if (typeof methods[methodOrOptions] === 'function') {
-        return methods[methodOrOptions].apply(this, args)
+    const instance = this.data('fbInstance')
+    if (instance[methodOrOptions]) {
+      if (typeof instance[methodOrOptions] === 'function') {
+        return instance[methodOrOptions].apply(this, args)
       }
-      return methods[methodOrOptions]
+      return instance[methodOrOptions]
     }
   } else {
-    const instance = methods.init(methodOrOptions, this)
-    Object.assign(methods, instance)
-    return instance
+    const plugin = new pluginInit(methodOrOptions, this)
+    this.data('fbInstance', plugin.instance)
+    return plugin.instance
   }
 }
diff --git a/tests/form-builder.test.js b/tests/form-builder.test.js
@@ -390,4 +390,50 @@ describe('FormBuilder can return formData', () => {
 
     expect(fb.actions.getData('xml')).toEqual('<form-template xmlns="http://www.w3.org/1999/xhtml"><fields><field type="header" subtype="h1" label="MyHeader" access="false"></field><field type="textarea" required="false" label="Comments" class-name="form-control" name="textarea-1696482495077" access="false" subtype="textarea"></field></fields></form-template>')
   })
+})
+
+describe('async loading tests', () => {
+  test('Will be log uninitialised errors if actions are called until the plugin has initialised', async () => {
+    const errorLogSpy = jest.spyOn(console, 'error').mockImplementation(() => {})
+    const infoLogSpy = jest.spyOn(console, 'info').mockImplementation(() => {})
+    const fbWrap = $('<div>')
+    const fb = $(fbWrap).formBuilder()
+    fb.actions.getData()
+    expect(errorLogSpy).toHaveBeenCalledWith('formBuilder is still initialising')
+
+    await fb.promise
+    fb.actions.getData()
+    expect(errorLogSpy).toHaveBeenCalledTimes(1)
+  })
+
+  test('Can load multiple formBuilders concurrently via promise interface without interference', async () => {
+    const wrap1 = $('<div>')
+    const wrap2 = $('<div>')
+    const p1 = wrap1.formBuilder().promise
+    const p2 = wrap2.formBuilder().promise
+
+    const fb1 = await p1
+    const fb2 = await p2
+
+    const field = {
+      type: 'text',
+      class: 'form-control'
+    }
+    fb1.actions.addField(field)
+
+    expect(fb1.actions.getData()).toHaveLength(1)
+    expect(fb2.actions.getData()).toHaveLength(0)
+    expect(fb1.formData).toHaveLength(96)
+    expect(fb2.formData).toHaveLength(2)
+
+    fb2.actions.addField(field)
+    fb2.actions.addField(field)
+
+    expect(wrap1.formBuilder('getData')).toHaveLength(1)
+    expect(wrap2.formBuilder('getData')).toHaveLength(2)
+    expect(wrap1.formBuilder('formData')).toHaveLength(96)
+    expect(wrap2.formBuilder('formData')).toHaveLength(191)
+
+    expect(wrap2.formBuilder('markup', 'div').outerHTML).toBe('<div></div>')
+  })
 })