Skip to content
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

Core: Fixes deprecated calls to jQuery trim. Fixes #2327 #2328

Conversation

@brighton1101
Copy link
Contributor

@brighton1101 brighton1101 commented May 7, 2020

This fixes the issue with deprecation and adds the polyfill for String.prototype.trim for browser support.

n8b
n8b approved these changes May 7, 2020
Copy link

@n8b n8b left a comment

Looks good to me!

src/core.js Outdated
@@ -1,3 +1,12 @@
// JQuery trim is deprecated, polyfill native js method if not defined
if ( !String.prototype.trim ) {
Copy link
Member

@staabm staabm May 7, 2020

Choose a reason for hiding this comment

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

We should not pollute the prototype of builtin objects. Please instead create a local trim-function instead

Copy link
Contributor Author

@brighton1101 brighton1101 May 7, 2020

Choose a reason for hiding this comment

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

Fixed.

src/core.js Outdated
@@ -1,3 +1,10 @@
// JQuery trim is deprecated, provide a trim method based on String.prototype.trim
function trim( str ) {
Copy link
Member

@staabm staabm May 7, 2020

Choose a reason for hiding this comment

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

Please move the function into a local variable and move it down to the functions where it is used

Copy link
Contributor Author

@brighton1101 brighton1101 May 7, 2020

Choose a reason for hiding this comment

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

Done.

staabm
staabm approved these changes May 7, 2020
Copy link
Member

@staabm staabm left a comment

Lgtm - thanks

@staabm staabm requested a review from Arkni May 7, 2020
@brighton1101
Copy link
Contributor Author

@brighton1101 brighton1101 commented May 12, 2020

@staabm @Arkni Any updates? We need to update to JQuery 3.5.1 and depend on this change being made

Arkni
Arkni approved these changes May 12, 2020
Copy link
Member

@Arkni Arkni left a comment

LGTM, Thanks.

@staabm
Copy link
Member

@staabm staabm commented May 12, 2020

@Arkni could you take care of merging this and tagging a new release?

@Arkni
Copy link
Member

@Arkni Arkni commented May 12, 2020

@staabm, I will try my best doing it this night or tomorrow.

@Arkni Arkni merged commit d3748a2 into jquery-validation:master May 13, 2020
@Arkni
Copy link
Member

@Arkni Arkni commented May 13, 2020

@staabm

My machine is behaving strangely and I couldn't run jquery-release for some reason. It may take me some time to investigate the issue and create a release. Could you please take over the task?

Thank you and sorry for the inconvenience.

@staabm staabm self-assigned this May 14, 2020
@staabm
Copy link
Member

@staabm staabm commented May 23, 2020

trying to do a release now

@Arkni
Copy link
Member

@Arkni Arkni commented May 27, 2020

Thanks a lot @staabm for the release :)

(And sorry for not being active. I'm still in a long pause from a lot of things including open source. It may take me a few more months to be able to help triaging issues or review PRs the way I used to.)

@staabm
Copy link
Member

@staabm staabm commented May 27, 2020

@Arkni no worries. Take your time - there are a lot of things which are more important then this project.

Best wishes

@bytestream
Copy link
Contributor

@bytestream bytestream commented Jun 22, 2020

This doesn't resolve all deprecations. Will send another PR

@bytestream
Copy link
Contributor

@bytestream bytestream commented Jun 23, 2020

Created #2335

@Tandulje
Copy link

@Tandulje Tandulje commented Aug 12, 2020

Hi...i'am using jquery -v3.5.1 and jquery.validate -v1.9.2

My code is
var settings = $('form').validate().settings;
settings.rules['UserPrincipalName'].required = true;

it gives error
VM155:294 Uncaught TypeError: Cannot set property 'required' of undefined

When i was on older version jquey -v2.1.4 then it was working fine but now not working with jquey- v3.5.1

Please give some solution on it.
I go through lots of link but not getting related to this.
Please let me know how can i fix this issue.

@bytestream
Copy link
Contributor

@bytestream bytestream commented Aug 12, 2020

@Tandulje please open a new issue, this is nothing to do with this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants