Skip to content
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

remove empty :not selectors (simpler alternative to PR #446) #448

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

fabianonunes
Copy link
Contributor

@fabianonunes fabianonunes commented Dec 2, 2020

Fixes #353
Fixes #370 (partially)

This is simpler alternative to PR #446. It only removes empty :not() selectors and doesn't try to fix dangling commas

After depseudify, a :not pseudo-class may become an empty :not(). Calling document.querySelector with an empty :not raises an exception and the entire selector is kept, because jsdom#findAll optimistically ignores all errors.

This PR adds a second pass to dePseudify: after removal of ignoredPseudos, a new walk is performed in search of empty :not selectors.

Current behavior:

  • dePseudify[.control:not(:checked)] => .control:not()
    • In this scenario, document.querySelector('.control:not()') raises an exception and the selector is kept.

Pull proposal:

  • dePseudify[.control:not(:checked)] => .control
    • No empty :not()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Doesn't remove :not selectors Unused selectors being kept
1 participant