NotificationsYou must be signed in to change notification settings
Fork20.5k
Star59.7k

Deprecated: Remove jQuery.trim#4695

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

mgol merged 1 commit intojquery:masterfrommgol:trim-removal

May 18, 2020

Merged

Deprecated: Remove jQuery.trim#4695

mgol merged 1 commit intojquery:masterfrommgol:trim-removal

May 18, 2020

+0 −28

Conversation

Copy link

Member

mgol commentedApr 29, 2020

Summary

The API has been deprecated in 3.5.0 so it can be removed in 4.0.0.

Refgh-4461

Checklist

All authors have signed the CLA athttps://cla.js.foundation/jquery/jquery
~~New tests have been added to show the fix or feature works~~
Grunt build and unit tests pass locally with these changes
If needed, a docs issue/PR was created athttps://github.com/jquery/api.jquery.com

Deprecated: Remove jQuery.trim

a260eb0

The API has been deprecated in 3.5.0 so it can be removed in 4.0.0.Refjquerygh-4461

mgol added Core Behavior Change labels

Apr 29, 2020

mgol added this to the4.0.0 milestone

Apr 29, 2020

mgol added the Needs review label

Apr 29, 2020

mgol self-assigned this

Apr 29, 2020

roiLeo approved these changes

Apr 30, 2020

View reviewed changes

timmywil approved these changes

Apr 30, 2020

View reviewed changes

Copy link

Member

timmywil left a comment

Choose a reason for hiding this comment

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

LGTM

mgol added the Discuss in MeetingReserved for Issues and PRs that anyone would like to discuss in the weekly meeting. label

May 5, 2020

dmethvin approved these changes

May 9, 2020

View reviewed changes

Copy link

Member

dmethvin left a comment

Choose a reason for hiding this comment

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

LGTM. I know there was some discussion thatjQuery.trim(s) is not equivalent tos.trim() whens is something other than a string. However, those are non-documented uses andString.prototype.apply(s) should work for those in any case since that's basically what we are doing here.

Copy link

MemberAuthor

mgol commentedMay 11, 2020

@dmethvinString.prototype.trim.apply(s) works ifs is a number or an object, etc. However, it throws ifs isnull orundefined. So it's not a complete replacement.

Maybe we need to update the note in Migrate?

Copy link

Member

dmethvin commentedMay 12, 2020

We can just add a note in thewarnings.md there explaining that something likeString.prototype.trim.apply(s==null? "" : s) works perhaps? In any case I think we can handle it in Migrate.