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

Blacklist harmful properties instead of whitelisting good ones #3

Open
kristerkari opened this issue Dec 13, 2017 · 4 comments
Open

Comments

@kristerkari
Copy link

Thanks for this plugin, I really like the idea behind it!

One thing that I thought that could be improved is that currently there is a whitelist of the allowed properties, but that's not really enough since there are some other properties that are not harmful. That is why I was thinking that it would be better to blacklist the harmful properties instead.

I was looking at the article that is mentioned in the readme (link), and it has a link to list of properties that cause either paint or layout:
https://docs.google.com/spreadsheets/d/1Hvi0nu2wG3oQ51XRHtMv-A_ZlidnwUYwgQsPQUg1R2s/pub?single=true&gid=0&output=html

There are some properties, e.g. filter that can be used for CSS transition that do not cause layout or paint.

The blacklist could be copied from there and this plugin could even have options if you want to allow paint props but warn for layout props.

What do you think @konstantin24121 ?

@konstantin24121
Copy link
Owner

@kristerkari hm, this is a good suggestion, i don't think about this. We can use something option for using recomended preset. But i've whitelist in plugin already

const allowedValue = ['opacity', 'transform', 'none', 'initial', 'inherit'].concat(options ? options.ignore : []);

I can add more safe values into him. It seems I forgot some properties

@kristerkari
Copy link
Author

kristerkari commented Mar 19, 2018

@konstantin24121 Yes, but my point was that a blacklist would work better than a whitelist, because with the current implementation you need to be updating the values always when browsers implement something new.

If you already know all the properties that are bad (cause layout), then create a blacklist of them and allow everything else not to cause a warning.

@konstantin24121
Copy link
Owner

Black list may be updated to, whitelist shorter :) I guess it'll not update very often

@kristerkari
Copy link
Author

kristerkari commented Mar 20, 2018

ok, if you want to continue using a whitelist, then you should update the whitelist based on these lists of animatable CSS properties, because the current whitelist is missing a lot of CSS properties:

The problem with whitelisting is that you don't know if all the properties that you whitelist cause layout or not.

With a blacklist you would know if a property that you add to the list causes layout, so you're only marking the properties that you know will cause layout to flag a warning.

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

No branches or pull requests

2 participants