enketo-core icon indicating copy to clipboard operation
enketo-core copied to clipboard

Discussion: remove jQuery

Open MartijnR opened this issue 5 years ago • 2 comments

Issues:

  • jquery instantiation and .find() function looks at all matches, and sometimes you only need the first -> inefficient
  • size of library

Proposed solution:

  • replace with home-made $() (single node) and $$() (multiple nodes).
  • create tiny library for used shortcuts where they make sense (.get(), .toggleClass, addClass, removeClass, .css(), .on(), .off() ...make inventory... ). No need for .each(), .map() etc. The rationale is basically to have an easy way to change multiple nodes and deal with events.

MartijnR avatar Jul 30 '18 17:07 MartijnR

I'm broadly in favour of this:

  • if it will give performance improvements, then that's fantastic
  • inclusion of jQuery occasionally gives us problems @medic when upgrading enketo due to version mismatches

Also interested in helping out - let me know if there's anything I can do.

alxndrsn avatar Aug 18 '18 12:08 alxndrsn

Thanks @alxndrsn!

I just finished doing this for the Model (#557) and it was less scary than I thought (took about 10 hours though). I think we may not have to create our own jQuery-like replacement. After the forthcoming move to ES6/7/8, I think the code will look clean and clear with map, reduce, forEach etc (and perhaps better - more explicit). So far I had to create just 1 utility function in dom-utils.js and some custom events in event.js.

I think for the remainder, we can perhaps use the following:

  • leave any jQuery-returning property/functions, that we suspect are used by apps, intact but add a console.deprecate warning and stop using them internally. We could then remove them (and jQuery) in version 5 #564 . (Maybe this applies only to form.view.$ - not sure)
  • leave the widgets dependency as a separate issue #518

I neglected to ask you to review the commits for #557, but if you want to do this or continue on with removing jQuery from Form.js, please do!

MartijnR avatar Sep 14 '18 20:09 MartijnR