- Notifications
You must be signed in to change notification settings - Fork20.6k
Selector: Makeselector.js module depend onattributes/attr.js#5384
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
selector.js module depend onattributes/attr.jsThe size report for this is pretty surprising... I could use some gzip magic from@gibson042 here. 😉 |
91fa203 to2a54f5dCompareAfter the latest changes:
|
36c55f1 tof192303CompareThis fixes custom builds using the `--include` switch that don't includethe `attributes` module.Fixesjquerygh-5379
f192303 tod7557b0CompareOK, without the addition of a dependency on so it looks this part is to blame. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
LGTM
| jQuery.each( ( | ||
| "checked selected async autofocus autoplay controls defer disabled " + | ||
| "hidden ismap loop multiple open readonly required scoped" | ||
| ).split( " " ), function( _i, name ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
| jQuery.each(( | |
| "checked selected async autofocus autoplay controls defer disabled "+ | |
| "hidden ismap loop multiple open readonly required scoped" | |
| ).split(" "),function(_i,name){ | |
| // HTML boolean attributes have special behavior: | |
| // we consider the lowercase name to be the only valid value, so | |
| // getting (if the attribute is present) normalizes to that, as does | |
| // setting to any non-`false` value (and setting to `false` removes the attribute). | |
| // See https://html.spec.whatwg.org/multipage/common-microsyntaxes.html#boolean-attributes | |
| jQuery.each(( | |
| "checked selected async autofocus autoplay controls defer disabled "+ | |
| "hidden ismap loop multiple open readonly required scoped" | |
| ).split(" "),function(_i,name){ |
Note also that our list is outdated; it is missing several boolean attributes fromHTML Attributes, includes ascoped that isn't even present there, and treatshidden as boolean despite the expansion of that attribute to be an enumeration including "until-found" bywhatwg/html@f5def65 .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Oh, interesting. Perhaps we should update this list for 4.0 as this can be considered a breaking change.
Or we should get rid of this manual normalization, depending on users doing the right thing here. The biggest issue are possibly falsy checks on values since an empty string is also a valid value. If we're worried about compat for such things, my guess would be we could limit this list to just a few entries likechecked &selected. Thoughts,@timmywil@gibson042?
EDIT: extracted to#5388, let's discuss this issue there
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I extracted the issue about the list itself into#5388; let's limit this PR to just the issue at hand and keep the list the same here.
Co-authored-by: Richard Gibson <richard.gibson@gmail.com>
Injquery/jquery#5384, `jQuery.expr.match.bool` stopped being defined and theregex matching boolean attributes is no longer exposed. Inline it in Migrateto avoid it crashing on jQuery 4.x.Fixesjquerygh-495
Injquery/jquery#5384, `jQuery.expr.match.bool` stopped being defined and theregex matching boolean attributes is no longer exposed. Inline it in Migrateto avoid it crashing on jQuery 4.x.Fixesgh-495Closesgh-496
Uh oh!
There was an error while loading.Please reload this page.
Summary
This fixes custom builds using the
--includeswitch that don't include theattributesmodule.Note: as of this change,
jQuery.expr.match.boolis no longer defined.Fixesgh-5379
Checklist