DEP Update jQuery to 3.x #1375

GuySartorelli · 2022-10-04T01:52:41Z

I've added some comments in this PR to help explain why some of the changes have been made.

A full CI run including behat for all modules which have a behat config has been run on kitchen sink. Note that running kitchen sink without this PR also has some failures (because cwp and other modules change defaults which change the test conditions) so compare these two to see if the failures are the same:

kitchen sink CI with PR: https://github.com/creative-commoners/recipe-kitchen-sink/actions/runs/3333342120
kitchen sink CI without PR: https://github.com/creative-commoners/recipe-kitchen-sink/actions/runs/3325921391

I've also run this against installer:

https://github.com/creative-commoners/silverstripe-installer/actions/runs/3333345210

I have also confirmed that unclecheese/display-logic still works

Parent issue

Update thirdparty jQuery libraries #1351

GuySartorelli · 2022-10-06T00:34:29Z

Functionality from client/src/legacy/LeftAndMain.menu.js has been removed because it isn't used, and was requiring an entire dependency (for a one-liner) which I would otherwise have had to update.

The example in that file doesn't even work as advertised - it gives the following output:

As you can see the styling for this is all over the place. It's a holdover from CMS 3 which actually broke part-way through CMS 3 itself, and hasn't been fixed or used since.

If anyone does want nested menus, there's a separate module for that.

Note: That module relies on some of this child menu functionality and related styling, so I won't remove all of it - but I will remove the "flyout" parts, which are not used at all.

GuySartorelli

The following deprecated jQuery functions were replaced with their modern counterparts throughout:

proxy replaced with the native Function.prototype.bind function
bind replaced with on
delegate replaced with on
live replaced with on
unbind replaced with on
undelegate replaced with on
andSelf replaced with addBack
trim replaced with the native String.prototype.trim function
isFunction replaced with comparing the result of typeof with 'function'.
isArray replaced with the native Array.isArray function
ajax.success, ajax.error, and ajax.complete replaced with ajax.done, ajax.fail, and ajax.always respectively

Though note that some of these are deprecated, but not yet removed - in those cases I have largely not replaced these in thirdparty scripts

GuySartorelli · 2022-10-18T03:29:21Z

client/src/bundles/vendor.js

-// require('../../../thirdparty/jquery-ui-themes/smoothness/jquery-ui.css');
-require('../../../thirdparty/jquery-entwine/dist/jquery.entwine-dist.js');
+require('../../../thirdparty/jquery-ui/jquery-ui.js'); // there is related css in styles/bundle.scss
+require('../../../thirdparty/jquery-entwine/jquery.entwine.js');


Moved because we only need this one file - all the rest is unnecessary cruft.

GuySartorelli · 2022-10-18T03:30:53Z

client/src/bundles/vendor.js

 require('jquery-sizes/lib/jquery.sizes.js');
-require('../../../thirdparty/jstree/jquery.jstree.js');
-// require('../../../thirdparty/stree/themes/apple/style.css');
-require('../../../thirdparty/jquery-hoverIntent/jquery.hoverIntent.js');


This dependency was only used in one place, and the place it was used was for legacy CMS 3 code (refer to #1375 (comment))

GuySartorelli · 2022-10-18T03:32:13Z

client/src/components/Menu/Menu.scss

@@ -220,7 +220,6 @@
    padding-top: 17px;
  }

-  .child-flyout-indicator,


For all the flyout removals, refer to #1375 (comment)

GuySartorelli · 2022-10-18T03:37:37Z

client/src/legacy/LeftAndMain.ActionTabSet.js

-            padding = activeTab.offset().top - containerSouth.offset().top;
+            const offset = activeTab.offset();
+            padding = offset ? (offset.top - containerSouth.offset().top) : 0;


This change is required because offset() returns undefined for hidden elements.

While it is possible to get the coordinates of elements with visibility:hidden set, display:none is excluded from the rendering tree and thus has a position that is undefined.

GuySartorelli · 2022-10-18T03:42:18Z

client/src/legacy/LeftAndMain.BatchActions.js

+      const selected = $(e.target).val();
+
+      // When resetting the field after a successful action, there won't be a valid "selected" option.
+      if (selected) {


See the onsubmit() function - the action is intentionally reset to the default option after a successful action has been fired (self.find(':input[name=Action]').val('').change();).

This works correctly before this PR - not sure what changed but this started throwing errors on the next line because selected was null or undefined. This avoids that issue.

GuySartorelli · 2022-10-18T04:24:34Z

thirdparty/jquery-entwine/jquery.entwine.js

-	var version = $.prototype.jquery.split('.');
-	var callbackIdx = (version[0] > 1 || version[1] >= 10 ? 1 : 2);
-
-	// Monkey patch $.fn.domManip to catch all regular jQuery add element calls


domManip wasn't even documented - it's an internal function that was removed in 3.x so we can't patch that to see when new nodes are being added. Instead, we now use mutations.

GuySartorelli · 2022-10-18T04:25:24Z

thirdparty/jquery-entwine/jquery.entwine.js

-		return _domManip.apply(this, arguments);
-	}
-
-	// Monkey patch $.fn.html to catch when jQuery sets innerHTML directly


This was always a dangerous way to detect this - but thankfully it isn't necessary because setting innerHTML will result in new nodes in the mutation observer.

GuySartorelli · 2022-10-18T04:27:24Z

thirdparty/jquery-entwine/jquery.entwine.js

-(function($) {	
-
-	/** Taken from jQuery 1.5.2 for backwards compatibility */
-	if ($.support.changeBubbles == undefined) {


$.support is deprecated but we don't need this anyway - it's to support old browsers we don't care about.

GuySartorelli · 2022-10-18T04:29:33Z

thirdparty/jquery-ondemand/jquery.ondemand.js

- * Requires jQuery 1.5 ($.Deferred support)
- *
- * CAUTION: Relies on customization of the 'beforeSend' callback in jQuery.ajaxSetup()


This doesn't require jQuery 1.5. It's been running on 1.7 for who knows how long, but I've now upgraded it to work with 3.6 - $.deferred is still very much supported

The note about beforeSend is unnecessary and frankly misleading here, since that customisation is included in this file anyway.

GuySartorelli · 2022-10-18T04:38:30Z

thirdparty/jquery-query/jquery.query.js

 * jQuery.query - Query String Modification and Creation for jQuery
 * Written by Blair Mitchelmore (blair DOT mitchelmore AT gmail DOT com)
 * Licensed under the WTFPL (http://sam.zoy.org/wtfpl/).
 * Date: 2009/8/13
 *
 * @author Blair Mitchelmore
- * @version 2.1.7
+ * @version 2.2.3


This has basically just been updated with the latest copy I could find

GuySartorelli · 2022-10-19T02:29:05Z

client/src/legacy/LeftAndMain.js

-        jQuery.clean( [ data ], document, fragment, [] );
-        $data = $(jQuery.merge( [], fragment.childNodes ));
+        $data = $($.parseHTML(data, document, false));


jQuery.clean() is an undocumented function that was removed in 3.0 - but the functionality is effectively the same as $.parseHTML() (the latter of which does the jQuery.merge() for us, so we can skip that part).

GuySartorelli · 2022-10-25T02:02:55Z

client/src/legacy/LeftAndMain.js

+/**
+ * The URL to use for saving and loading tab state
+ */
+window.ss.tabStateUrl = function() {


This needs to be accessible in TabSet.js now

GuySartorelli · 2022-10-25T02:05:12Z

client/src/legacy/LeftAndMain.js

+          const tabset = $(this);
+          const tabsetId = tabset.attr('id');
+          const overrideState = (overrideStates && overrideStates[tabsetId]) ? overrideStates[tabsetId] : null;
+          tabset.restoreState(overrideState);


The state restore functionality has moved to TabSet.js so that each tab can be responsible for restoring its own state.
This is necessary because onAdd() is now triggered asyncronously some time after the element is added (see changes in entwine to using mutation observers) which means the tabs may not be initialised yet when this function is called.

GuySartorelli · 2022-10-25T02:06:30Z

client/src/legacy/TabSet.js

@@ -36,6 +40,46 @@ $.entwine('ss', function($){
        this._super();
    },

+    restoreState: function (overrideState) {


This functionality is mostly copied directly from LeftAndMain.js with some minor changes:

some of the code has been slightly modernised

if the tabset hasn't been initialised yet, is is marked to defer restoring the state until redrawTabs() is called.

GuySartorelli · 2022-10-26T03:15:45Z

client/src/legacy/LeftAndMain.Tree.js

-          .bind('select_node.jstree check_node.jstree uncheck_node.jstree', function(e, data) {
+          .on('select_node.jstree check_node.jstree uncheck_node.jstree', function(e, data) {
+            // Clear out namespace before triggering so entwine handlers are called
+            e.namespace = '';


This didn't need to be set to blank in the past because of a bug in jQuery where the namespace wasn't actually kept on the event after triggerHandler() was called. This bug has been fixed in the current version of jQuery, so we have to manually remove the namespace for it to get to entwine.

GuySartorelli · 2022-10-26T23:33:06Z

client/src/legacy/LeftAndMain.EditForm.js

-    ValidationErrorShown: false,
+    getValidationErrorShown: function() {
+      return Boolean(this.data('_validationErrorShown'));
+    },
+    setValidationErrorShown: function(value) {
+      this.data('_validationErrorShown', value);
+    },


Turning this from an entwine property into regular javascript data means it is tied to the actual DOM element, which means we can rely on it being reset when a new DOM element is added.

GuySartorelli · 2022-10-26T23:34:22Z

client/src/legacy/LeftAndMain.EditForm.js

@@ -96,9 +98,6 @@ $.entwine('ss', function($){
        }
      }

-      // Reset error display
-      this.setValidationErrorShown(false);


We can't rely on this being reset BEFORE we fire a validation message, since onadd is fired asyncronously to the redrawTabs() call which triggers a call to onafterredrawtabs() which relies on this value.

lekoala · 2022-11-24T08:33:06Z

i'm so glad the team finally decided to upgrade jquery :-) that was a pleasant surprise while reading the changelog

NightJar · 2023-04-19T00:04:32Z

I have also confirmed that unclecheese/display-logic still works

@GuySartorelli I'm curious to what version you tested against.
I'm using 2.0.6 (latest 2 line, released Feb '23) and I'm seeing instant errors

Uncaught TypeError: s.default.entwine is not a function in http://localhost/_resources/vendor/unclecheese/display-logic/client/dist/js/bundle.js?m=1676431318

I'm trying to figure out if there's too much jQuery, an issue with display logic's front end bundle, or it's actually an issue with the change in interface between these two modules.

GuySartorelli · 2023-04-19T01:07:14Z

I'm curious to what version you tested against.

It was way too long ago for me to remember it, and I didn't note down the version at the time, unfortunately.

NightJar · 2023-04-19T02:09:37Z

I'm curious to what version you tested against.

It was way too long ago for me to remember it, and I didn't note down the version at the time, unfortunately.

That's cool, I thought that might be the case; thanks for responding.
I've been able to identify that the previously deployed display-logic version (2.0.5) still exhibits the issue, meaning it's not a regression (it worked fine in active service with CMS 4.11).

I'm busy trying to assert it's not the fault of my site before I open an issue :)
jQuery is loading, but isn't Entwine isn't binding to it for some reason, although it loads immediately afterward (in normal old school jQuery fashion).

NightJar · 2023-04-19T02:50:12Z

I'm trying to figure out if there's too much jQuery

✅
... It was debugbar 🤦
Site is fine in test/live mode 😄

This was referenced Oct 4, 2022

Update thirdparty jQuery libraries #1351

Closed

DEP Update thirdparty jQuery libaries #1352

Closed

ENH Upgrade jquery to 1.12 #1353

Closed

POC JQuery 3 support in CMS4 #1357

Closed

GuySartorelli force-pushed the pulls/1/jquery3-upgrade branch 9 times, most recently from 783fb58 to 2749dce Compare October 5, 2022 23:43

GuySartorelli force-pushed the pulls/1/jquery3-upgrade branch 3 times, most recently from 1d335aa to 427991f Compare October 18, 2022 03:28

GuySartorelli commented Oct 18, 2022

View reviewed changes

GuySartorelli force-pushed the pulls/1/jquery3-upgrade branch 5 times, most recently from 78b96df to 9419c07 Compare October 19, 2022 00:54

GuySartorelli commented Oct 19, 2022

View reviewed changes

GuySartorelli force-pushed the pulls/1/jquery3-upgrade branch 4 times, most recently from 4f3151b to c93420b Compare October 25, 2022 02:02

GuySartorelli commented Oct 25, 2022

View reviewed changes

GuySartorelli force-pushed the pulls/1/jquery3-upgrade branch from c93420b to a39ede4 Compare October 26, 2022 03:12

GuySartorelli commented Oct 26, 2022

View reviewed changes

GuySartorelli force-pushed the pulls/1/jquery3-upgrade branch 4 times, most recently from 4986131 to 6c0deb1 Compare October 26, 2022 23:31

GuySartorelli commented Oct 26, 2022

View reviewed changes

GuySartorelli force-pushed the pulls/1/jquery3-upgrade branch from 6c0deb1 to 3f65bd3 Compare October 27, 2022 00:52

DEP Update jQuery to 3.x

914b69d

GuySartorelli force-pushed the pulls/1/jquery3-upgrade branch from 3f65bd3 to 914b69d Compare October 27, 2022 01:07

GuySartorelli marked this pull request as ready for review October 27, 2022 01:13

emteknetnz approved these changes Nov 1, 2022

View reviewed changes

emteknetnz merged commit be31300 into silverstripe:1 Nov 1, 2022

emteknetnz deleted the pulls/1/jquery3-upgrade branch November 1, 2022 04:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DEP Update jQuery to 3.x #1375

DEP Update jQuery to 3.x #1375

GuySartorelli commented Oct 4, 2022 •

edited

Loading

GuySartorelli commented Oct 6, 2022 •

edited

Loading

GuySartorelli left a comment •

edited

Loading

GuySartorelli Oct 18, 2022

GuySartorelli Oct 18, 2022

GuySartorelli Oct 18, 2022

GuySartorelli Oct 18, 2022

GuySartorelli Oct 18, 2022

GuySartorelli Oct 18, 2022

GuySartorelli Oct 18, 2022

GuySartorelli Oct 18, 2022

GuySartorelli Oct 18, 2022

GuySartorelli Oct 18, 2022

GuySartorelli Oct 19, 2022 •

edited

Loading

GuySartorelli Oct 25, 2022

GuySartorelli Oct 25, 2022

GuySartorelli Oct 25, 2022

GuySartorelli Oct 26, 2022

GuySartorelli Oct 26, 2022

GuySartorelli Oct 26, 2022

lekoala commented Nov 24, 2022

NightJar commented Apr 19, 2023

GuySartorelli commented Apr 19, 2023

NightJar commented Apr 19, 2023

NightJar commented Apr 19, 2023

DEP Update jQuery to 3.x #1375

DEP Update jQuery to 3.x #1375

Conversation

GuySartorelli commented Oct 4, 2022 • edited Loading

Parent issue

GuySartorelli commented Oct 6, 2022 • edited Loading

GuySartorelli left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GuySartorelli Oct 19, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lekoala commented Nov 24, 2022

NightJar commented Apr 19, 2023

GuySartorelli commented Apr 19, 2023

NightJar commented Apr 19, 2023

NightJar commented Apr 19, 2023

GuySartorelli commented Oct 4, 2022 •

edited

Loading

GuySartorelli commented Oct 6, 2022 •

edited

Loading

GuySartorelli left a comment •

edited

Loading

GuySartorelli Oct 19, 2022 •

edited

Loading