Strange error behavior of event delegation when customizing jQuery

[ajiho]

Description

My build script is as follows:

npm run build -- --esm --include=event --include=traversing

When I use event delegation

html

<button data-test1="test1" data-test2="test2">test</button>

$(document).on('click', '[data-test1][data-test2]', function () {
    //error: Uncaught TypeError: jQuery.attr is not a function
});

According to the description of readme.md,selector: The full jQuery selector engine. When this module is excluded, it is replaced with a rudimentary selector engine based on the browser's querySelectorAll,However, querySelectorAll supports this type of selector
I would like to ask if this design is intentional or is it a flaw?

By the way, I have already tested it in the full version and there is no such issue

Thank you.

Link to test case

[mgol]

Thanks for the report. This is on the main branch, isn't it? That's code for future jQuery 4.0 which is not stable yet. When building for production use, it's recommended to check out the latest tag and build there.

In jQuery 3.x, the ATTR filter uses find.attr:

jquery/src/selector.js

Line 1057 in 87467a6

var result = find.attr( elem, name );

defined in the same file:

jquery/src/selector.js

Lines 853 to 876 in 87467a6

	find.attr = function( elem, name ) {
	// Set document vars if needed
	// Support: IE 11+, Edge 17 - 18+
	// IE/Edge sometimes throw a "Permission denied" error when strict-comparing
	// two documents; shallow comparisons work.
	// eslint-disable-next-line eqeqeq
	if ( ( elem.ownerDocument || elem ) != document ) {
	setDocument( elem );
	}
	var fn = Expr.attrHandle[ name.toLowerCase() ],
	// Don't get fooled by Object.prototype properties (see trac-13807)
	val = fn && hasOwn.call( Expr.attrHandle, name.toLowerCase() ) ?
	fn( elem, name, !documentIsHTML ) :
	undefined;
	if ( val !== undefined ) {
	return val;
	}
	return elem.getAttribute( name );
	};

On the main branch, I removed the selector version and started using jQuery.attr:

jquery/src/selector.js

Line 454 in c98597e

var result = jQuery.attr( elem, name );

which lies outside of this module and this dependency was not made explicit.

Depending on attr.js is a bit hard because that module depends on selector itself:

jquery/src/attributes/attr.js

Line 7 in c98597e

import "../selector.js";

but that's mostly an artifact from jQuery 3.x previously depending on jQuery.find.attr from the selector module here:

jquery/src/attributes/attr.js

Line 68 in 87467a6

ret = jQuery.find.attr( elem, name );

while on the main branch a native getAttribute is used:

jquery/src/attributes/attr.js

Line 61 in c98597e

ret = elem.getAttribute( name );

We should remove the dependency on selector.js from attr.js. As for fixing the selector issue, we have then two options:

1. Move from using jQuery.attr to the native getAttribute directly - this is more in line with what jQuery 3.x was doing as the find.attr wrapper was in most cases deferring to the native method. However, that avoids attrHooks for attribute matching which is consulted when using the jQuery attr method.
2. Add the dependency on attr.js to selector.js. That will make attrHooks respected.

I'm gravitating towards the first option for the following reasons:

1. It's more in line with the current selector behavior that has been followed for many years.
2. There are not that many built-in attrHooks left on the main branch. There's one for type but that's just for IE and only a setter (selector only needs getters) and there's one for boolean attributes - this latter one may actually constitute a breaking change as the value for all boolean attributes is always its lowercase name.

[mgol]

Actually, I was wrong about the logic change. Expr.attrHandle used to be populated in src/attr.js in 3.x:

jquery/src/attributes/attr.js

Line 13 in b922eed

attrHandle = jQuery.expr.attrHandle;

jquery/src/attributes/attr.js

Line 13 in b922eed

attrHandle = jQuery.expr.attrHandle;

On the 4.x line, this has been moved to be a regular member of jQuery.attrHooks:

jquery/src/attributes/attr.js

Lines 102 to 128 in 99151d7

	jQuery.each( jQuery.expr.match.bool.source.match( /\w+/g ), function( _i, name ) {
	jQuery.attrHooks[ name ] = {
	get: function( elem ) {
	var ret,
	isXML = jQuery.isXMLDoc( elem ),
	lowercaseName = name.toLowerCase();
	if ( !isXML ) {
	ret = elem.getAttribute( name ) != null ?
	lowercaseName :
	null;
	}
	return ret;
	},
	set: function( elem, value, name ) {
	if ( value === false ) {
	// Remove boolean attributes when set to false
	jQuery.removeAttr( elem, name );
	} else {
	elem.setAttribute( name, name );
	}
	return name;
	}
	};
	} );

Thus, the logic is fine, we just need to declare the dependency explicitly.

I submitted removing unneeded selector.js dependencies in PR #5383, I'll submit a separate one to add the dependency on attr.js to selector.js.

[mgol]

PR: #5384