Event: Warn about late use of $(window).on("load"...)

dmethvin · dmethvin · commit 1776e3d29382 · 2016-08-05T17:45:13.000-04:00
Fixes #200
diff --git a/src/event.js b/src/event.js
@@ -1,4 +1,5 @@
 var oldLoad = jQuery.fn.load,
+	oldEventAdd = jQuery.event.add,
 	originalFix = jQuery.event.fix;
 
 jQuery.event.props = [];
@@ -35,6 +36,15 @@ jQuery.event.fix = function( originalEvent ) {
 	return fixHook && fixHook.filter ? fixHook.filter( event, originalEvent ) : event;
 };
 
+jQuery.event.add = function( elem, types ) {
+
+	// This misses the multiple-types case but that seems awfully rare
+	if ( elem === window && types === "load" && document.readyState === "complete" ) {
+		migrateWarn( "$(window).on('load'...) called after load event occurred" );
+	}
+	return oldEventAdd.apply( this, arguments );
+};
+
 jQuery.each( [ "load", "unload", "error" ], function( _, name ) {
 
 	jQuery.fn[ name ] = function() {
diff --git a/test/event-lateload.html b/test/event-lateload.html
@@ -0,0 +1,37 @@
+<!DOCTYPE html>
+<html>
+<head>
+	<meta charset="utf-8">
+	<title>jQuery Migrate late load event binding test</title>
+
+	<!-- Load a jQuery and jquery-migrate plugin file based on URL -->
+	<script src="testinit.js"></script>
+	<script>
+		TestManager.loadProject( "jquery", "git" );
+	</script>
+	<script src="iframeTest.js"></script>
+	<script>
+		jQuery.noConflict();
+		TestManager.loadProject( "jquery-migrate", "dev", true );
+	</script>
+	<script>
+		var loaded = jQuery.Deferred();
+
+		// No warning here, document isn't yet loaded
+		jQuery( window ).on( "load", function() {
+			loaded.resolve();
+		});
+
+		jQuery.when( jQuery.ready, loaded ).then( function() {
+
+			// This .on() call should warn
+			jQuery( window ).on( "load", jQuery.noop );
+
+			startIframeTest();
+		} );
+	</script>
+</head>
+<body>
+	<p>jQuery Migrate late load event binding test</p>
+</body>
+</html>
diff --git a/test/event.js b/test/event.js
@@ -145,3 +145,12 @@ TestManager.runIframeTest( "jQuery.event.fixHooks", "event-fixHooks.html",
 		assert.equal( jQuery.migrateWarnings.length, 2, "warnings: " +
 			JSON.stringify( jQuery.migrateWarnings ) );
 	} );
+
+TestManager.runIframeTest( "Load within a ready handler", "event-lateload.html",
+	function( assert, jQuery, window, document, log ) {
+		assert.expect( 2 );
+
+		assert.equal( jQuery.migrateWarnings.length, 1, "warnings: " +
+			JSON.stringify( jQuery.migrateWarnings ) );
+		assert.ok( /load/.test( jQuery.migrateWarnings[ 0 ] ), "message ok" );
+	} );
diff --git a/warnings.md b/warnings.md
@@ -193,3 +193,9 @@ See jQuery-ui [commit](https://github.com/jquery/jquery-ui/commit/c0093b599fcd58
 **Cause:** The code on the page has used the `jQuery.event.props` or `jQuery.event.fixHooks` data structures. These were used in previous versions to affect the properties that are copied from the native event to the jQuery event each time an event is delivered, but they had the potential to create performance issues. Versions of jQuery Mobile before 1.5 make use of this API and require jQuery Migrate to run properly.
 
 **Solution:** The most popular use of these data structures are to add properties for touch or pointer events, and those properties are now supported by default with a newer high-performance approach in jQuery 3.0 that only retrieves the properties on first access. If you are using jQuery Mobile, check the [jquerymobile.com](https://jquerymobile.com) site for updates. If you are using plugins such as [pointerTouch](https://github.com/timmywil/jquery.event.pointertouch) or [touchHooks](https://github.com/aarongloege/jquery.touchHooks), simply remove them as they are no longer needed.
+
+### JQMIGRATE: $(window).on('load'...) called after load event occurred
+
+**Cause:** The calling code has attempted to attach a `load` event to `window` after the page has already loaded. That means the handler will never run and so is probably not what the caller intended. This can occur when the event attachment is made too late, for example, in a jQuery ready handler. It can also occur when a file is loaded dynamically with jQuery after the page has loaded, for example using the `$.getScript()` method.
+
+**Solution:** If a function `fn` does not actually depend on all page assets being fully loaded, switch to a ready handler `$( fn )` which runs earlier and will aways run `fn` even if the script that contains the code loads long after the page has fully loaded. If `fn` actually does depend on the script being fully loaded, check `document.readyState`. If the value is `"complete"` run the function immediately, otherwise use `$(window).on( "load", fn )`.
