timwis / jkan

A lightweight, backend-free open data portal, powered by Jekyll
https://jkan.io
MIT License
218 stars 310 forks source link

Remove jquery #241

Open timwis opened 1 year ago

timwis commented 1 year ago

Nearly finished, but not quite — just pushing this up as a draft for visibility.

Still need to:

Note that it will be worth comparing bundle sizes before merging, to verify whether this PR adds value (tree shaking has come a long way)

timwis commented 1 year ago

Bundle size comparison

We could shave off another large chunk (probably 60 KB) by getting tree shaking working with lodash, but it's not immediately possible because of the use of the library's chain method (which pulls in the whole library, as I understand it), so would require a refactor in two of the components.

Last thing I want to do on this branch is just check the browser support to make sure we're comfortable with the changes.

BryanQuigley commented 1 year ago

Was looking to review this, but it looks like bootstrap 5 dropped JQuery anyway. Does this mean we would just stick on bootstrap instead of bootstrap.native?

timwis commented 1 year ago

Yep, that's right. To be honest, I got to the end of this pull request and just felt like it made readability worse without much benefit. Happy to carry on with it once we've got bootstrap 5 ready, though I'm half-tempted to just rewrite it (perhaps in stimulus, which takes a similar multi-page app approach) since it's so little JS at this point.