- Issue created by @catch
- Status changed to Postponed
10 months ago 11:30am 7 February 2024 - π¬π§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 .
- Status changed to Needs review
10 months ago 11:50pm 8 February 2024 - π¬π§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.
- π¬π§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.
- Status changed to Needs work
10 months ago 2:19pm 10 February 2024 - Status changed to Needs review
10 months ago 6:07pm 10 February 2024 - π¦πΊ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 2:00pm 4 March 2024 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 3:44pm 4 March 2024 - π¬π§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.
- π«π·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
- Status changed to RTBC
9 months ago 2:26pm 6 March 2024 - πΊπΈ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 8e1ef1e7 on 10.3.x
-
longwave β
committed a01b8c2a on 11.x
Issue #3419734 by catch, nod_: [jQuery 4] jquery-form is unmaintained...
-
longwave β
committed a01b8c2a on 11.x
- Status changed to Fixed
9 months ago 12:55pm 8 March 2024 - π¬π§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.
- Status changed to Needs work
6 months ago 9:33pm 14 May 2024 - Status changed to Needs review
6 months ago 9:44pm 14 May 2024 - π¬π§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 9:54pm 14 May 2024 - πΊπΈ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.