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 prefixing #18

Closed
corysimmons opened this issue Mar 31, 2015 · 15 comments
Closed

Remove prefixing #18

corysimmons opened this issue Mar 31, 2015 · 15 comments

Comments

@corysimmons
Copy link
Contributor

Everyone could/should be using Autoprefixer (or some similar/lesser tool) in their development environment. It's usually a bad practice to have prefixing in libraries.

@ConnorAtherton
Copy link
Owner

I didn't know it was bad practice. Could you elaborate?

I don't see the down side including prefixes here. In the case that they don't use autoprefixer (I agree, they should) the styles will work and if they do then it works anyway, no harm done.

@corysimmons
Copy link
Contributor Author

I used to think that way but I got chewed out by the community for doing it and now I'm a convert.

The idea is, it's not Loaders.css' job to prefix. It adds repo bloat for no benefit. I promise you won't have many issues from this change.

@ConnorAtherton
Copy link
Owner

Okay, I can see the sense in that. I've just uncommented it out for now.

@corysimmons
Copy link
Contributor Author

👍

@jpcmf
Copy link

jpcmf commented Apr 2, 2015

I dont use autoprefixer and I use SASS. Too bad remove the prefixer 👎

@corysimmons
Copy link
Contributor Author

You should be using Autoprefixer in your build process though. :\

@rictorres
Copy link

You are assuming that whoever uses this library must have a build process in their project.

Are you sure this is the correct approach?

Also, Autoprefixer will add or remove prefixes according to your settings.

@corysimmons
Copy link
Contributor Author

It's a pretty fair assumption to say that most people will have some sort of build process, and that in that build process they'll be using Autoprefixer.

On the flip side you're assuming they want prefixes baked in - I for one don't. I'd rather have personal control over what prefixes are added with my Autoprefixer settings rather than some heavy handed prefixes applied that may or may not be relevant in a month.

@rictorres
Copy link

I disagree. Imagine jQuery publishing a ES6-only version hoping that most users are using Babel or similar.

If you're using Autoprefixer, you have personal control over the prefixes: it'll add or remove prefixes according to your settings :)

@corysimmons
Copy link
Contributor Author

Ultimately it's up to the maintainer of the lib. I'll personally never prefix a single library I create and continue assuming people are using Autoprefixer, and if they aren't, help them to use it.

@ConnorAtherton Some of your users have expressed concern about this change and I seem to be the only one against it, so maybe it's time to reconsider prefixes.

Here's some further reading about when I used to argue for prefixes and the community asking me to remove them:

Here's typically what happens when someone who isn't using Autoprefixer comes along (pretty rare):

@ConnorAtherton
Copy link
Owner

Thanks for the input guys. I especially appreciate the links @corysimmons.

My original thoughts on this topic can be summed up with this comment. Apart from the extra bloat, I saw no harm adding prefixes for reasons outlined in that comment. And as @rictorres mentioned earlier, you still have control over which prefixes to keep and which to remove.

It requires extra effort to add a build system to run through Autoprefixer whereas it requires no effort to run it through your current build system because you will have already configured the settings.

The main argument against adding prefixes by default is that the library would then be making decisions it shouldn't, as well as violating the single responsibility principle. I think that's a fair argument if you assume that most - if not everyone - using the library will be running it through some build step to add the prefixes before using it. From my experience with the node community, this is the case. However, I'm not sure about other other communities out there and I don't want to rule people out from using this just because I assumed they had a particular build step in their system.

Adding them does add extra bloat but I think in this instance this is justified because the unprefixed browser support is so low (22.3%).

For now, I want to keep them in and revisit this in the future when I've asked around a bit more.

@corysimmons
Copy link
Contributor Author

I personally disagree with this decision, but I think it's a really good idea to listen to your users so 👍

@rictorres
Copy link

good call, @ConnorAtherton 👍

@jpcmf
Copy link

jpcmf commented Apr 9, 2015

Don't worry about me, I just remove the loaders.css from my project :)

@mustaqsheikh
Copy link

I agree with rictorres.

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

5 participants