From 199c1cd2601594b84c305c17a0c62fb81e51d9d4 Mon Sep 17 00:00:00 2001 From: James Lucas Date: Thu, 19 Oct 2023 16:50:30 +1100 Subject: [PATCH] fix: Refactor initialisation of the formBuilder plugin to ensure that two or more concurrent initialisations cannot interfere with each other --- src/js/form-builder.js | 112 +++++++++++++++++++------------------ tests/form-builder.test.js | 46 +++++++++++++++ 2 files changed, 104 insertions(+), 54 deletions(-) diff --git a/src/js/form-builder.js b/src/js/form-builder.js index 093eafb1a..e6ec708a5 100644 --- a/src/js/form-builder.js +++ b/src/js/form-builder.js @@ -2489,68 +2489,72 @@ 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 - }, + const actionList = [ + 'getFieldTypes', + 'addField', + 'clearFields', + 'closeAllFieldEdit', + 'getData', + 'removeField', + 'save', + 'setData', + 'setLang', + 'showData', + 'showDialog', + 'toggleAllFieldEdit', + 'toggleFieldEdit', + 'getCurrentFieldId', + ] + + this.instance = { + actions: actionList.reduce((actions, currentAction) => { actions[currentAction] = notInitialised; return actions }, {}), + 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 index 0738286f1..8294280b3 100644 --- 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('') }) +}) + +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 = $('
') + 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 = $('
') + const wrap2 = $('
') + 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('
') + }) }) \ No newline at end of file