🌐 AI搜索 & 代理 主页
Skip to content

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.

Sign up for GitHub

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

Jump to bottom
Merged
mgol merged 1 commit into from
May 18, 2020
Merged

Deprecated: Remove jQuery.trim #4695

mgol merged 1 commit into from
May 18, 2020

Conversation

@mgol
Copy link
Member

@mgol mgol commented Apr 29, 2020

Summary

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

Ref gh-4461

Checklist

@mgol
Deprecated: Remove jQuery.trim
a260eb0 
The API has been deprecated in 3.5.0 so it can be removed in 4.0.0.

Ref jquerygh-4461
@mgol mgol added this to the 4.0.0 milestone Apr 29, 2020
@mgol mgol self-assigned this Apr 29, 2020
timmywil
timmywil approved these changes Apr 30, 2020
View reviewed changes
Copy link
Member

@timmywil 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 mgol added the Discuss in Meeting Reserved for Issues and PRs that anyone would like to discuss in the weekly meeting. label May 5, 2020
dmethvin
dmethvin approved these changes May 9, 2020
View reviewed changes
Copy link
Member

@dmethvin 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 that jQuery.trim(s) is not equivalent to s.trim() when s is something other than a string. However, those are non-documented uses and String.prototype.apply(s) should work for those in any case since that's basically what we are doing here.

@mgol
Copy link
Member Author

mgol commented May 11, 2020

@dmethvin String.prototype.trim.apply(s) works if s is a number or an object, etc. However, it throws if s is null or undefined. So it's not a complete replacement.

Maybe we need to update the note in Migrate?

@dmethvin
Copy link
Member

dmethvin commented May 12, 2020

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

@mgol
Copy link
Member Author

mgol commented May 12, 2020

@dmethvin that sounds good!

@mgol
Copy link
Member Author

mgol commented May 18, 2020

@dmethvin I submitted a PR with the text change: jquery/jquery-migrate#358.

@mgol mgol removed Discuss in Meeting Reserved for Issues and PRs that anyone would like to discuss in the weekly meeting. Needs review labels May 18, 2020
@mgol mgol merged commit 0b676ae into jquery:master May 18, 2020
@mgol mgol deleted the trim-removal branch May 18, 2020 21:20
@marvin-min marvin-min mentioned this pull request Mar 17, 2021
Open
@abotteram abotteram mentioned this pull request Oct 26, 2021
Closed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 7, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@dmethvin dmethvin dmethvin approved these changes

@timmywil timmywil timmywil approved these changes

+1 more reviewer

@roiLeo roiLeo roiLeo approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@mgol mgol

Labels

Behavior Change Core

Milestone

4.0.0

Development

Successfully merging this pull request may close these issues.

4 participants

@mgol @dmethvin @timmywil @roiLeo