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.

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

Only warn against removeAttr(booleanAttr) if property value not false #471

Closed
mgol opened this issue Aug 11, 2022 · 0 comments · Fixed by #484
Closed

Only warn against removeAttr(booleanAttr) if property value not false #471

mgol opened this issue Aug 11, 2022 · 0 comments · Fixed by #484
Assignees
@mgol
Milestone
3.4.1

Comments

@mgol
Copy link
Member

mgol commented Aug 11, 2022

Migrate warns against .removeAttr() called on boolean attributes as this used to set the associated property to false but it doesn't do so anymore since jQuery 3.0.0.

However, this is only an issue if the property wasn't already set to false. There are valid reasons to call removeAttr on boolean attributes and it shouldn't cause issues as long as the property is being updated first.

This is not just a theoretical concern - AngularJS sets both boolean properties & then associated attributes, calling removeAttr if the value passed was false. See a report about that at angular/angular.js#16877. I also hit it at my employer when updating jQuery.

The proposal is to read the property and if it's already false, skip the warning.
@mgol mgol added this to the 3.4.1 milestone Aug 11, 2022
@mgol mgol self-assigned this Aug 11, 2022
@mgol mgol changed the title Only warn against `removeAttr(booleanAttr) Only warn against removeAttr(booleanAttr) Aug 19, 2022
@mgol mgol changed the title Only warn against removeAttr(booleanAttr) Only warn against removeAttr(booleanAttr) if property value not false Feb 21, 2023
@mgol mgol mentioned this issue Feb 23, 2023
Merged
mgol added a commit to mgol/jquery-migrate that referenced this issue Feb 23, 2023
@mgol
Attributes: Don't warn against removeAttr if property false
79e384b 
Some frameworks, like AngularJS, invoke `removeAttr` on boolean attributes
but only after setting the respective property to `false`. Such usage is not
dangerous and would continue to work even after Migrate is removed. Therefore,
only warn for `removeAttr(booleanAttribute)` if the property wasn't `false`
already.

Fixes jquerygh-471
Closes jquerygh-484
@mgol mgol closed this as completed in #484 Feb 23, 2023
mgol added a commit that referenced this issue Feb 23, 2023
@mgol
Attributes: Don't warn against removeAttr if property false
072e44d 
Some frameworks, like AngularJS, invoke `removeAttr` on boolean attributes
but only after setting the respective property to `false`. Such usage is not
dangerous and would continue to work even after Migrate is removed. Therefore,
only warn for `removeAttr(booleanAttribute)` if the property wasn't `false`
already.

Fixes gh-471
Closes gh-484
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mgol mgol

Labels
None yet
Projects
None yet
Milestone
3.4.1
Development

Successfully merging a pull request may close this issue.

Attributes: Don't warn against removeAttr if property false mgol/jquery-migrate
1 participant
@mgol