-
Notifications
You must be signed in to change notification settings - Fork 7
Fixes unwanted throw in hasAllAttributes
#59
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
Conversation
|
I think this could work well. That line has been there from the beginning, not sure what the reasoning behind it is, but it might be useful in some cases where you write the wrong attribute name for something? But yeah, i think it can be removed without any issues. |
|
@runegan Thanks for the review. On a similar note, it throws when the selector is |
|
The intention behind a check like this is to help the developer not use an impossible selector on the DOM. If it’s removed you lose that safety net. CSS selectors don't have this check and it is okay b/c the dom can change a lot. This isn’t true in AE. Personally I say keep it but I don’t use it enough to see it as a problem either. |
|
Another solution is to make "strict checking" an option on select that defaults to |
|
I'm not the right decision maker for this. It should come from someone that
uses the library more. That said, I still have the same opinion as before :)
…On Thu, Sep 5, 2024 at 11:56 AM Zack Lovatt ***@***.***> wrote:
@rafikhan <https://github.com/rafikhan> @Klustre
<https://github.com/Klustre> @runegan <https://github.com/runegan> Any
feelings on this, a year later? :)
—
Reply to this email directly, view it on GitHub
<#59 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAJ5W6BD7P5OSKNOHII7LDZVCSM7AVCNFSM6AAAAABNXDMAJOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMZSGQZTCMBRGQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
I don't use selectors at all, so also not able to chime in with usage thoughts. |
|
Same opinion; it doesn't make sense that it throws. I also haven't used aequery in the last 2 years. But the reasoning behind it is pretty simple: when you use the query selector to get all layers with |
|
The person that used it cares. So maybe drop it?
…On Thu, Sep 5, 2024 at 12:25 PM Remco Janssen ***@***.***> wrote:
Same opinion; it doesn't make sense that it throws. I also haven't used
aequery in the last 2 years.
—
Reply to this email directly, view it on GitHub
<#59 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAJ5W7RGGXB7ZPFBRL4NL3ZVCV3PAVCNFSM6AAAAABNXDMAJOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMZSGQ3TOOJXGA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
While using
aeq(SelectorString)it threw an error if the object didn't have the prop I was filtering (guideLayer=truewhich isn't present on Camera Layers for instance). So it broke the selector instead of silently ignoring it.