Skip to content

Conversation

@mgol
Copy link
Member

@mgol mgol commented Jan 29, 2024

Summary

The attrHooks entries for boolean attributes are only defined for jQuery 4+; jQuery 3.x used a separate mechanism - assigning them to jQuery.expr.attrHandle. That object used to be maintained by Sizzle, since jQuery 3.7.0 it's kept in the selector module. Because of that, the isXMLDoc check used to be require in this hook.

Now that standard attrHooks are used, the isXMLDoc check already happens inside of jQuery.attr and there's no need to repeat it in the test. Note that this repetition is even incorrect - while Sizzle's jQuery.find.attr used to treat an undefined output of the hooks from jQuery.expr.attrHandle as a way to opt out of the hook, jQuery's attrHooks use null to opt out of a getter hook.

Apart from the size, this patch also avoids unnecessary extra checks.

Size difference:

main @805cdb43fd02c3a5783c06b5ec2c9519be0682ab
   raw     gz Filename
   -35    -12 dist/jquery.min.js
   -35    -15 dist/jquery.slim.min.js
   -35    -15 dist-module/jquery.module.min.js
   -35    -15 dist-module/jquery.slim.module.min.js

Checklist

The `attrHooks` entries for boolean attributes are only defined for jQuery 4+;
jQuery 3.x used a separate mechanism - assigning them to
`jQuery.expr.attrHandle`. That object used to be maintained by Sizzle, since
jQuery 3.7.0 it's kept in the selector module. Because of that, the `isXMLDoc`
check used to be require in this hook.

Now that standard `attrHooks` are used, the `isXMLDoc` check already happens
inside of `jQuery.attr` and there's no need to repeat it in the test. Note that
this repetition is even incorrect - while Sizzle's `jQuery.find.attr` used to
treat an `undefined` output of the hooks from `jQuery.expr.attrHandle` as a way
to opt out of the hook, jQuery's `attrHooks` use `null` to opt out of a getter
hook.

Apart from the size, this patch also avoids unnecessary extra checks.
@mgol mgol merged commit b40a480 into jquery:main Jan 31, 2024
@mgol mgol deleted the attrHooks-simplification branch January 31, 2024 00:47
@mgol mgol added this to the 4.0.0 milestone Jan 31, 2024
@mgol mgol removed the Needs review label Jan 31, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 24, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Development

Successfully merging this pull request may close these issues.

3 participants