From c89540f72a395fd0a6c3648f12b8cddac3d97275 Mon Sep 17 00:00:00 2001 From: sullivanst Date: Fri, 27 May 2016 18:48:22 -0400 Subject: [PATCH 1/6] Simplify integration with reporting tools We use Raygun to log our javascript errors, and we'd like to automatically report migration warnings there too. Raising an event when a warning is logged seems a very natural way to do this - we can come scrape $.migrateWarnings when triggered. A stack trace also seems like a very useful thing to have, so extending the warning seems like a natural way to achieve this (it also helps a lot with Raygun, which likes to send Error objects). --- src/migrate.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/migrate.js b/src/migrate.js index 0c391f7d..bf90fce9 100644 --- a/src/migrate.js +++ b/src/migrate.js @@ -47,13 +47,16 @@ function migrateWarn( msg ) { var console = window.console; if ( !warnedAbout[ msg ] ) { warnedAbout[ msg ] = true; - jQuery.migrateWarnings.push( msg ); + var msgObj = new String(msg); + msgObj.asError = new Error(msg); + jQuery.migrateWarnings.push( msgObj ); if ( console && console.warn && !jQuery.migrateMute ) { console.warn( "JQMIGRATE: " + msg ); if ( jQuery.migrateTrace && console.trace ) { console.trace(); } } + jQuery(window).trigger("migrateWarning"); } } From 8698dd6418262e092840534edb4e2aeb47b61d4d Mon Sep 17 00:00:00 2001 From: sullivanst Date: Fri, 27 May 2016 19:00:27 -0400 Subject: [PATCH 2/6] Make it lint Lint rules do not allow use of new String(), so use a parallel array to store Errors rather than a property on a String object. --- src/migrate.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/migrate.js b/src/migrate.js index bf90fce9..ad80a1cb 100644 --- a/src/migrate.js +++ b/src/migrate.js @@ -31,6 +31,7 @@ var warnedAbout = {}; // List of warnings already given; public read only jQuery.migrateWarnings = []; +jQuery.migrateWarnings.asErrors = []; // Set to false to disable traces that appear with warnings if ( jQuery.migrateTrace === undefined ) { @@ -40,16 +41,15 @@ if ( jQuery.migrateTrace === undefined ) { // Forget any warnings we've already given; public jQuery.migrateReset = function() { warnedAbout = {}; - jQuery.migrateWarnings.length = 0; + jQuery.migrateWarnings.length = jQuery.migrateWarnings.asErrors.length = 0; }; function migrateWarn( msg ) { var console = window.console; if ( !warnedAbout[ msg ] ) { warnedAbout[ msg ] = true; - var msgObj = new String(msg); - msgObj.asError = new Error(msg); - jQuery.migrateWarnings.push( msgObj ); + jQuery.migrateWarnings.push( msg ); + jQuery.migrateWarnings.asErrors.push( msg ); if ( console && console.warn && !jQuery.migrateMute ) { console.warn( "JQMIGRATE: " + msg ); if ( jQuery.migrateTrace && console.trace ) { From 6f78e58bb3360fb7127ec4767886c7b7cd3b723d Mon Sep 17 00:00:00 2001 From: sullivanst Date: Fri, 27 May 2016 19:02:25 -0400 Subject: [PATCH 3/6] Fix spacing for lint style --- src/migrate.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/migrate.js b/src/migrate.js index ad80a1cb..b0db808d 100644 --- a/src/migrate.js +++ b/src/migrate.js @@ -56,7 +56,7 @@ function migrateWarn( msg ) { console.trace(); } } - jQuery(window).trigger("migrateWarning"); + jQuery( window ).trigger( "migrateWarning" ); } } From 02c4f38626fde9a0e1fd6dc6ae0d9a811f7bda2c Mon Sep 17 00:00:00 2001 From: sullivanst Date: Fri, 27 May 2016 19:14:01 -0400 Subject: [PATCH 4/6] Add check for warning as Errors --- test/migrate.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/migrate.js b/test/migrate.js index 314fa4a6..c99f80d1 100644 --- a/test/migrate.js +++ b/test/migrate.js @@ -19,6 +19,7 @@ function expectWarning( name, expected, fn ) { // Simple numeric equality assertion for warnings matching an explicit count } else if ( expected && jQuery.migrateWarnings.length === expected ) { equal( jQuery.migrateWarnings.length, expected, name + ": warned" ); + equal( jQuery.migrateWarnings.asErrors.length, expected, name + ": warned as Errors" ); // Simple ok assertion when we saw at least one warning and weren't looking for an explict count } else if ( !expected && jQuery.migrateWarnings.length ) { From b72995efdc33240f56a9621dcd865c997bc5eeb9 Mon Sep 17 00:00:00 2001 From: Matthew Whelan Date: Fri, 27 May 2016 19:35:55 -0400 Subject: [PATCH 5/6] Tests: get passing Update tests to account for additional assert in expectWarning with a specified, non-zero count --- test/ajax.js | 2 +- test/attributes.js | 4 ++-- test/core.js | 2 +- test/data.js | 2 +- test/event.js | 4 ++-- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/test/ajax.js b/test/ajax.js index 81b1ed09..12712e91 100644 --- a/test/ajax.js +++ b/test/ajax.js @@ -1,7 +1,7 @@ module( "ajax" ); test( "jQuery.ajax() deprecations on jqXHR", function( assert ) { - assert.expect( 3 ); + assert.expect( 4 ); var done = assert.async(); diff --git a/test/attributes.js b/test/attributes.js index 90810f22..d8562bb3 100644 --- a/test/attributes.js +++ b/test/attributes.js @@ -2,7 +2,7 @@ QUnit.module( "attributes" ); QUnit.test( ".removeAttr( boolean attribute )", function( assert ) { - assert.expect( 8 ); + assert.expect( 9 ); expectNoWarning( "non-boolean attr", function() { var $div = jQuery( "
" ) @@ -43,7 +43,7 @@ QUnit.test( ".removeAttr( boolean attribute )", function( assert ) { } ); QUnit.test( ".toggleClass( boolean )", function( assert ) { - assert.expect( 14 ); + assert.expect( 17 ); var e = jQuery( "
" ).appendTo( "#qunit-fixture" ); diff --git a/test/core.js b/test/core.js index a0fec314..646b5deb 100644 --- a/test/core.js +++ b/test/core.js @@ -28,7 +28,7 @@ test( "jQuery( '#' )", function() { } ); QUnit.test( "Attribute selectors with unquoted hashes", function( assert ) { - expect( 31 ); + expect( 34 ); var markup = jQuery( "
" + diff --git a/test/data.js b/test/data.js index 2dd834b7..53e8899b 100644 --- a/test/data.js +++ b/test/data.js @@ -16,7 +16,7 @@ test( "jQuery.data() camelCased names", function( assert ) { "-dashy-hanger" ]; - assert.expect( 16 ); + assert.expect( 17 ); var curData, div = document.createElement( "div" ); diff --git a/test/event.js b/test/event.js index b70f9797..7d47e716 100644 --- a/test/event.js +++ b/test/event.js @@ -53,7 +53,7 @@ test( "load() and unload() event methods", function( assert ) { } ); QUnit.test( ".bind() and .unbind()", function( assert ) { - assert.expect( 3 ); + assert.expect( 5 ); var $elem = jQuery( "
" ).appendTo( "#qunit-fixture" ); @@ -73,7 +73,7 @@ QUnit.test( ".bind() and .unbind()", function( assert ) { } ); QUnit.test( ".delegate() and .undelegate()", function( assert ) { - assert.expect( 3 ); + assert.expect( 5 ); var $div = jQuery( "
" ).appendTo( "#qunit-fixture" ); From 97eddc0af4870d3ab806db6b08c9924b31d8e061 Mon Sep 17 00:00:00 2001 From: sullivanst Date: Fri, 27 May 2016 20:12:47 -0400 Subject: [PATCH 6/6] Add missing Error wrapper migrateWarnings.asErrors was supposed to wrap the message in an Error message to create a stack trace --- src/migrate.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/migrate.js b/src/migrate.js index b0db808d..b2686987 100644 --- a/src/migrate.js +++ b/src/migrate.js @@ -49,7 +49,7 @@ function migrateWarn( msg ) { if ( !warnedAbout[ msg ] ) { warnedAbout[ msg ] = true; jQuery.migrateWarnings.push( msg ); - jQuery.migrateWarnings.asErrors.push( msg ); + jQuery.migrateWarnings.asErrors.push( new Error( msg ) ); if ( console && console.warn && !jQuery.migrateMute ) { console.warn( "JQMIGRATE: " + msg ); if ( jQuery.migrateTrace && console.trace ) {