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

Prevent duplicate selectors being added to reset ruleset #18

Merged
merged 2 commits into from
Oct 9, 2016
Merged

Prevent duplicate selectors being added to reset ruleset #18

merged 2 commits into from
Oct 9, 2016

Conversation

simonsmith
Copy link
Contributor

If a selector appeared more than once (media query, nesting) it was being duplicated in the reset ruleset. This change adds a check for the selector first.

cc @giuseppeg

@giuseppeg
Copy link

Since we are in ES6 I would use a Set instead of an Array – access with has is O(1) and you won't need the extra contains helper

@simonsmith
Copy link
Contributor Author

That's a good shout but I'd need to add the babel polyfill and I wonder if it's worth the extra code.

@giuseppeg
Copy link

@simonsmith ok, your call! the changes looks good to me

@maximkoretskiy
Copy link
Owner

@simonsmith Could you add changes description into unrealesed section of changelog

@simonsmith
Copy link
Contributor Author

@maximkoretskiy Done

@simonsmith
Copy link
Contributor Author

I'll just rebase this with your latest changes

It's common to have duplicate selectors in media queries or nested
selectors. This change checks to see if the selector has already been
added and skips it if so.
@maximkoretskiy
Copy link
Owner

@simonsmith sorry. Just pushed fix. could you resolve the conflict?

@maximkoretskiy maximkoretskiy merged commit 46a86f4 into maximkoretskiy:master Oct 9, 2016
@maximkoretskiy
Copy link
Owner

Just published your changes to npm. Thank U guys!

@simonsmith simonsmith deleted the duplicate-selector branch October 9, 2016 20:41
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.

3 participants