-
-
Notifications
You must be signed in to change notification settings - Fork 97
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
Omit tagName from nth-child selector as it is redundant #68
base: master
Are you sure you want to change the base?
Omit tagName from nth-child selector as it is redundant #68
Conversation
I'm publishing this PR to see if the automatic tests fail (they won't run for me on my local machine) |
So from the tests. Changes to the next: - script:nth-child(8)
+ body > :nth-child(8) - .cell:nth-child(1) > p:nth-child(3)
+ .-gutters > :nth-child(1) > :nth-child(3) So this change will not make selectors shorter in all cases. Sometimes the selector will become longer. More interesting solution will be to add support for :nth-of-type. |
Ah okay, I've got the tests running locally so can have a better play around with it. On a more semantic note, I think relying on the intersection of position and tagName, while it might produce a shorter selector is very brittle. E.g.
If a span is later inserted (in a different version of the page, or as modifications are made to the page), then we are now referencing a different element:
So applying this PR is maybe a judgement call either in favour of robustness or brevity, would you agree? |
True. I'm not sure what path is better yet. One concern I have: imagine selector on .foo :nth-child(0) , later some other tag added. The selector is still valid but points to a wrong element. But .foo div:nth-child(0) will break (better to fail fast). |
Yes, but you are acknowledging that redundancy is good. There are plenty of other times when finder doesn't produce the required level of redundancy for the situation (e.g. my example above). I've some further ideas with which to update this PR, but for now have produced #70 as the next step, so no point in continuing this discussion if that one is unacceptable! |
I've some parts of |
This is another way of asking the following question which I asked in #3