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

didRetain() instead of result.sels #46

Closed
leeoniya opened this issue Apr 18, 2020 · 2 comments
Closed

didRetain() instead of result.sels #46

leeoniya opened this issue Apr 18, 2020 · 2 comments
Labels
enhancement New feature or request

Comments

@leeoniya
Copy link
Owner

i never quite liked the naming of result.sels or that it gets accumulated regardless of whether the user asked for it or not. it also doesnt feel right that whitelist accumulation uses this return style but whitelist testing uses the shouldDrop() callback style. didRetain() which fires for every undropped selector feels more consistent.

this would be a breaking change but to a non-primary API, so i dont think a 2.0 is warranted; hopefully no-one's life depends on dropcss 😱

@leeoniya leeoniya added the enhancement New feature or request label Apr 18, 2020
@eemeli
Copy link

eemeli commented Jul 20, 2020

I noticed this change while refactoring some code. Dropping sels would for us cause things to break stuff. This API is also documented in the README's example code.

I'd recommend you either bump the version to 2.0, or add the new API in parallel with the old.

@leeoniya
Copy link
Owner Author

yes, i'll likely bump it to 2.0.

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

No branches or pull requests

2 participants