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

Created on 7 February 2024, 10 months ago
Updated 28 May 2024, 6 months 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

jQuery Form is a low-level JavaScript library used internally to handle sending data back to Drupal over Ajax. The library is abandoned upstream but has been forked into core β†’ (and removed as an external dependency) so that we can upgrade to jQuery 4 in Drupal 11. It is tagged @internal and should not be used by contributed or custom modules directly.

πŸ“Œ Task
Status

Fixed

Version

10.3 ✨

Component
JavascriptΒ  β†’

Last updated 2 days 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 10 months 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 β†’ (Closed) created by catch
  • Merge request !6519Draft: Resolve #3419734 "With jquery4 update" β†’ (Closed) created by catch
  • Status changed to Needs review 10 months 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
    10 months 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 🌱 [Plan] 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
    10 months ago
    Total: 683s
    #91222
  • πŸ‡¬πŸ‡§United Kingdom catch
  • Pipeline finished with Failed
    10 months ago
    #91502
  • Pipeline finished with Failed
    10 months ago
    Total: 634s
    #91596
  • Pipeline finished with Failed
    10 months ago
    Total: 470s
    #91662
  • Status changed to Needs work 10 months ago
  • πŸ‡«πŸ‡·France nod_ Lille
  • Pipeline finished with Success
    10 months ago
    Total: 618s
    #92044
  • Pipeline finished with Success
    10 months ago
    Total: 600s
    #92074
  • Pipeline finished with Failed
    10 months ago
    Total: 579s
    #92077
  • Status changed to Needs review 10 months ago
  • πŸ‡¬πŸ‡§United Kingdom catch

    Addressed @nod_'s comments.

  • Pipeline finished with Failed
    10 months ago
    Total: 550s
    #92086
  • Pipeline finished with Success
    10 months 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.

  • Status changed to Needs work 9 months ago
  • The Needs Review Queue Bot β†’ tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

    This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

    Consult the Drupal Contributor Guide β†’ to find step-by-step guides for working with issues.

  • Status changed to Needs review 9 months ago
  • πŸ‡¬πŸ‡§United Kingdom catch

    Rebased. Two weeks without any response on https://github.com/jquery-form/form/issues/616, which is not really a surprise given the general inactivity in the repo, so I think we should continue here.

  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    That unit failure may just need a rebase?

  • Pipeline finished with Canceled
    9 months ago
    Total: 727s
    #112726
  • πŸ‡«πŸ‡·France nod_ Lille

    +1 to copy and maintain in core to resolve the jQuery 4 compatibility issue. We can always go back and use the upstream version if it ever updates.

    Added some more cleanup

  • Pipeline finished with Running
    9 months ago
    #112741
  • Status changed to RTBC 9 months ago
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    Seems to have the support of all the right people. And seems to be just moving a copy of jquery-form so think this is good.

    • longwave β†’ committed 8e1ef1e7 on 10.3.x
      Issue #3419734 by catch, nod_: [jQuery 4] jquery-form is unmaintained...
    • longwave β†’ committed a01b8c2a on 11.x
      Issue #3419734 by catch, nod_: [jQuery 4] jquery-form is unmaintained...
  • Status changed to Fixed 9 months ago
  • πŸ‡¬πŸ‡§United Kingdom longwave UK

    Discussed with @catch. We decided to backport so we can keep things in sync and either improve jquery.form.js in the future (it has support for IE and old versions of jQuery that can be stripped out if anyone wants to work on that) or continue work in πŸ“Œ Remove jQuery Form dependency from misc/ajax.js Needs work where we hopefully can remove it entirely. The library was already marked as internal in core.libraries.yml and while it underpins Drupal's AJAX form posts it should not be used directly by contrib or custom code so there should be no need for a change record or release note.

    Committed and pushed a01b8c2ad5 to 11.x and 8e1ef1e78a to 10.3.x. Thanks!

  • Automatically closed - issue fixed for 2 weeks with no activity.

  • πŸ‡ΊπŸ‡ΈUnited States xjm
  • Status changed to Needs work 6 months ago
  • πŸ‡ΊπŸ‡ΈUnited States xjm

    We need a release note for this.

  • πŸ‡ΊπŸ‡ΈUnited States xjm

    And a CR.

  • Status changed to Needs review 6 months ago
  • πŸ‡¬πŸ‡§United Kingdom longwave UK

    Added CR: https://www.drupal.org/node/3447202 β†’

    Summarised further in the release note snippet in the IS.

  • Status changed to Fixed 6 months ago
  • πŸ‡ΊπŸ‡ΈUnited States xjm

    Tweaked the release note a bit to clarify that it's a removed dep, and added to the draft.

    Thanks!

  • Automatically closed - issue fixed for 2 weeks with no activity.

Production build 0.71.5 2024