[jQuery 4] jquery-form is unmaintained and not jQuery 4 compatible, fork it into core

Created on 7 February 2024, 18 days ago
Updated 14 February 2024, 11 days ago

Problem/Motivation

This isn't our code so we need to fork or fix upstream or something.

Steps to reproduce

Proposed resolution

Merge request link

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

πŸ“Œ Task
Status

Needs review

Version

11.0 πŸ”₯

Component
JavascriptΒ  β†’

Last updated about 14 hours ago

Created by

πŸ‡¬πŸ‡§United Kingdom catch

Live updates comments and jobs are added and updated live.
  • JavaScript

    Affects the content, performance, or handling of Javascript.

Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @catch
  • Status changed to Postponed 18 days ago
  • πŸ‡¬πŸ‡§United Kingdom catch

    Postponing on πŸ“Œ Remove jQuery Form dependency from misc/ajax.js Needs work which we should try to get done first before thinking about a fallback.

  • πŸ‡¬πŸ‡§United Kingdom catch

    Also we can't do a straight swap for ES6 trim, see πŸ› Regression fix for (if feasible) uses of the jQuery trim function to use vanillaJS Fixed .

  • Merge request !6517Fork jquery.form.js β†’ (Open) created by catch
  • Merge request !6519Draft: Resolve #3419734 "With jquery4 update" β†’ (Open) created by catch
  • Status changed to Needs review 16 days ago
  • πŸ‡¬πŸ‡§United Kingdom catch

    Spent a bit of time on πŸ“Œ Remove jQuery Form dependency from misc/ajax.js Needs work and got it a bit further, but there is at least one or two methods we more or less have to copy over to ajax.js in that issue, and then I couldn't get it to work.

    So I thought I'd try a straight 'fork + update' in this issue, and that might be easier.

    MR copies the unminified jquery.form.js to /misc, updates library definitions, and makes the absolute minimum changes to skip eslint and update to jQuery 4.

    If we go down this route, we will want to fix the tax indentation and maybe not skip eslint (which will require dozens of fixes to make it compatible).

    Given we have an issue open to replace the entire AJAX system with HTMX, it might be best to do the minimal fork and update here after all, then we can try to rework all of this completely.

  • Pipeline finished with Success
    16 days ago
    Total: 601s
    #91210
  • πŸ‡¬πŸ‡§United Kingdom catch

    MR is green.

    I got a bit lost in πŸ“Œ Remove jQuery Form dependency from misc/ajax.js Needs work trying to solve the dozens of eslint failures which is why I took a break and tried this instead. I am not JavaScript developer so it might be me, but it's also a lot of churn so feels like it might not only be me.

    I'm not sure what the general direction should be, but some thoughts:

    If we want to do 🌱 Gradually replace Drupal's AJAX system with HTMX Active then I am not sure it is really worth finishing πŸ“Œ Remove jQuery Form dependency from misc/ajax.js Needs work or at least not making it a jQuery 4 blocker. The fieldValue() method is a fair amount of code to fork, so unless there's a drop-in replacement, we are forking a big chunk of jQuery form and also having to add bc layers and deprecations etc.

    If we do this issue, we need to decide if we will try to bring our forked jquery.form.js up to core's eslint standards - this will make diffs from the original nearly impossible if so, but if we don't it's a pain to work with. Even my minimal vim config is struggling with tabs vs spaces as you can see from the ugly diff.

    Another possibility would be to fork only the methods we still need from jQuery form into their own file for πŸ“Œ Remove jQuery Form dependency from misc/ajax.js Needs work , if that works it would make it clearer what we've forked and what is our own ajax.js. I might attempt this next over in that issue, maybe that will end up being the minimal amount of code churn and more maintainable solution, at least without new ideas.

    The actual goal here was to be able merge in the jQuery 4 update together with this MR to see if jquery-form is our only remaining blocker, and that's the case, so this in itself is very good news, and I think either this issue or the other one are viable solutions, it just needs someone more competent than me to have a look.

  • Pipeline finished with Failed
    16 days ago
    Total: 683s
    #91222
  • πŸ‡¬πŸ‡§United Kingdom catch
  • Pipeline finished with Failed
    16 days ago
    #91502
  • Pipeline finished with Failed
    16 days ago
    Total: 634s
    #91596
  • Pipeline finished with Failed
    15 days ago
    Total: 470s
    #91662
  • Status changed to Needs work 15 days ago
  • πŸ‡«πŸ‡·France nod_ Lille
  • Pipeline finished with Success
    15 days ago
    Total: 618s
    #92044
  • Pipeline finished with Success
    15 days ago
    Total: 600s
    #92074
  • Pipeline finished with Failed
    15 days ago
    Total: 579s
    #92077
  • Status changed to Needs review 15 days ago
  • πŸ‡¬πŸ‡§United Kingdom catch

    Addressed @nod_'s comments.

  • Pipeline finished with Failed
    15 days ago
    Total: 550s
    #92086
  • Pipeline finished with Success
    15 days ago
    Total: 535s
    #92087
  • πŸ‡¦πŸ‡ΊAustralia larowlan πŸ‡¦πŸ‡ΊπŸ.au GMT+10

    I note there's an active PR for jquery form with these changes - https://github.com/jquery-form/form/pull/599/files - is it worth us opening an issue in their queue to see if they have any 4.x plans or if they'd consider taking one or more of us on as maintainers?

  • πŸ‡¬πŸ‡§United Kingdom catch

    #11 good find. There is only a single maintainer for jquery-form and the last time they interacted with the project was 2020, so I did not have any hope of them responding to an issue. We could open one just so we've exhausted our options without forking, but also if we keep going with πŸ“Œ Remove jQuery Form dependency from misc/ajax.js Needs work we could end up entirely removing jQuery form in 11.x and maybe even in 10.x if we can get the bc layers right, so any co-maintainership would be very short lived in that case.

    @andypost it's GPL licensed so given Drupal is also GPL licensed I'm not sure that's necessary?

  • πŸ‡¬πŸ‡§United Kingdom catch

    I opened an issue against jquery-form asking about a new jQuery 4 compatible release: https://github.com/jquery-form/form/issues/616

    Just in case, but if that goes unanswered within the next week or two, I think we should proceed here given there's not much activity there in general any more.

Production build https://api.contrib.social 0.61.6-2-g546bc20