Skip to content

Modernize the build/test, only assign to the old API in 3.x #6

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
May 5, 2025

Conversation

mgol
Copy link
Collaborator

@mgol mgol commented Apr 15, 2025

Changes:

  1. Modernize the build/test infra: avoid the needless build, use jtr to run tests on real browsers, test browsers on CI.
  2. Only assign to the legacy API when jQuery 3.x is used.

@dmethvin BTW, if you'd like, I'd be happy to help maintain this plugin. If you agree, could you give me write access?

@mgol mgol changed the title Modernize the buid/test, fix Migrate issues Modernize the build/test, fix Migrate issues Apr 21, 2025
Changes:
1. Modernize the build/test infra: avoid the needless build, use jtr to run
   tests on real browsers, test browsers on CI.
2. Make the plugin not cause jQuery Migrate warnings due to assignment to the
   legacy API.
@mgol mgol marked this pull request as draft May 4, 2025 11:45
Comment on lines 28 to 41
var version = jQuery.fn.jquery
.split( "." )
.map( function( v ) {
return Number( v );
} );

// Only assign to the newer API if supported to avoid jQuery Migrate warnings.
if ( version[ 0 ] >= 4 || ( version[ 0 ] === 3 && version[ 1 ] >= 7 ) ) {
jQuery.Deferred.getErrorHook = getErrorHook;
} else {
jQuery.Deferred.getStackHook = getErrorHook;
}

return getErrorHook;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes can now mostly be reverted - or at least simplified - after jquery/jquery-migrate#577 & jquery/jquery-migrate#578.

Tests will need updates as well.

I'm considering assigning both in jQuery <4 and only the new API in jQuery >=4; that would be much shorter.

@mgol mgol marked this pull request as ready for review May 5, 2025 16:28
@mgol mgol changed the title Modernize the build/test, fix Migrate issues Modernize the build/test, only assign to the old API in 3.x May 5, 2025
@mgol mgol merged commit ce32a41 into dmethvin:master May 5, 2025
5 checks passed
@mgol mgol deleted the improvements branch May 5, 2025 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant