-
Notifications
You must be signed in to change notification settings - Fork 26
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
Why dasherize adds a leading dash? #23
Comments
I agree this is weird. However this functionality is based on underscore.string, and they actually specify it to work like this in their docs: http://epeli.github.io/underscore.string/#dasherize-string-gt-string I can see this being useful in their documented use case (special css properties). However I can also see a usecase for what you describe. Maybe we should change the default behaviour to do what you describe, and add a flag "addInitialDash" or something like that, to keep the initial dash functionality accessible. This will however be an API break, so we'd have to release a major version for this. @lorenzo what do you think? |
I see, thanks for answering. The use case they show for css properties is very specific. I don't think this library should to that as is not the general behaviour you would expect. If you want a leading dash then is trivial to add a prefix later. |
@sporto can you send a PR fixing this? |
I would also be okay with just removing the initial dash and have the user add it themselves later, however I think we should make sure to mention that when releasing it as a major version |
One potential approach for versioning: add the new behavior as |
@lorenzo I'll be happy to make a PR. Re alternative name to avoid major version. Making a major version change seems like a big deal, I struggle with this in my libraries. But since using Elm I'm getting more comfortable with doing major version changes for seemingly small things, but still feels heavy. Up to you. |
I'm ok with a new major version |
Doesn't add _ at the beginning.
Adds leading
_
.What is the logic for this. Seems unexpected.
The text was updated successfully, but these errors were encountered: