Skip to content

Added camelCase option #149

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

Merged
merged 2 commits into from
Oct 19, 2015
Merged

Added camelCase option #149

merged 2 commits into from
Oct 19, 2015

Conversation

nkt
Copy link
Contributor

@nkt nkt commented Sep 27, 2015

This patch helps developers use css-modules with external libraries, like bootstrap. Popular css frameworks use kebab-case (bootstrap, foundation), or maybe other, like snake_case. We can resolve this problem if would automatically convert them to camelCase.

/* styles.css */
.btn-info {
  color: blue;
}
// app.js
const styles = require('!css?modules&camelCase!./styles.css');

console.log(styles.btnInfo) // _38r5hlPyrqKLodJwWOdM1k
console.log(styles['btn-info']) // undefined

@blia
Copy link

blia commented Sep 27, 2015

👍

@nkbt
Copy link

nkbt commented Sep 27, 2015

👍 nice

@nkt
Copy link
Contributor Author

nkt commented Oct 11, 2015

@sokra ping

@blia
Copy link

blia commented Oct 14, 2015

Can You keep both versions(original, camelCase) in result scope?
For intuitive using as strings in bound scope?
Example:

/* styles.css */
.btn-info {
  color: blue;
}
.is-loading {
  color: red;
}
// app.js
const styles = require('!css?modules&camelCase!./styles.css');
const cx = require('classnames/bind').bind(styles);
// as strings
const classNamesA = cx('btn-info', 'is-loading');
// as logical notation
const classNamesB = cx('btn-info', { isLoading : true });
const isLoading = true;
const classNamesC = cx('btn-info', { isLoading });
console.log(classNamesA) // =>  "__btn-info___xCfnx __is-loading___3sOhl"
console.log(classNamesB) // =>  "__btn-info___xCfnx __is-loading___3sOhl"
console.log(classNamesC) // =>  "__btn-info___xCfnx __is-loading___3sOhl"

@b2whats
Copy link

b2whats commented Oct 14, 2015

👍

@chicoxyzzy
Copy link

great feature

@nkt
Copy link
Contributor Author

nkt commented Oct 18, 2015

@sokra ping

@sokra
Copy link
Member

sokra commented Oct 18, 2015

@nkt yes, great feature. Could you keep both versions(original, camelCase) in result scope as @blia suggested?

@nkt nkt force-pushed the camelcased-keys branch from 97fddae to 5126cff Compare October 18, 2015 22:55
@nkt
Copy link
Contributor Author

nkt commented Oct 18, 2015

@sokra done

sokra added a commit that referenced this pull request Oct 19, 2015
@sokra sokra merged commit d1aa291 into webpack-contrib:master Oct 19, 2015
@sokra
Copy link
Member

sokra commented Oct 19, 2015

Thanks

@nkt nkt deleted the camelcased-keys branch October 19, 2015 13:14
@faergeek
Copy link
Contributor

@nkt @sokra It somehow doesn't work for css/locals loader (i.e. on server)

@faergeek
Copy link
Contributor

It should be move from loader.js to processCss.js probably?

@nkt
Copy link
Contributor Author

nkt commented Oct 22, 2015

@faergeek yep. I'm working on this.

@faergeek
Copy link
Contributor

Cool, it should probably just be factored out from loader.js to use it from both localLoader.js and loader.js?.

Cool feature, BTW 👍

@faergeek
Copy link
Contributor

Would be cool to have possibility to use camelCase which replaces only dashes, leaving underscores alone, what do you think?

@nkt
Copy link
Contributor Author

nkt commented Oct 22, 2015

@faergeek now it's keeping both variants.

@faergeek
Copy link
Contributor

I mean if we have class like menu-item_disabled (BEM naming) it should result in menuItem_disabled instead of menuItemDisabled to allow it's usage as identifier in js, but still distinguish modifiers from elements :-)

@faergeek
Copy link
Contributor

I could contribute this functionality into your fork

@faergeek
Copy link
Contributor

Or I could just make a separate PR which will add it to css/locals loader and add option for camelCasing if you're busy now

nkt pushed a commit to nkt/css-loader that referenced this pull request Oct 22, 2015
@nkt
Copy link
Contributor Author

nkt commented Oct 22, 2015

@faergeek I've published new PR.
I guess you're suggesting another mode of camel-casing, so you can add it as option:
"loader": "style!css?modules&camelCase" - already contributed camelCase behavior, standard.
"loader": "style!css?modules&camelCase=bem" - new behavior specially for BEM.

@faergeek
Copy link
Contributor

Yes, but probably use another name for that or just alias it (camelcase=dashes-only or camelcase=identifiers or something like that may be?).

@sokra
Copy link
Member

sokra commented Oct 22, 2015

Just don't use BEM when using CSS Modules. That makes no sense. Instead of menu-item_disabled use disabled. You don't need namespacing in your class names, it's namespaced by the file.

@faergeek
Copy link
Contributor

@sokra I think it's still convenient to distinguish between element and modifier.
CSS Modules eliminate the usefulness of blocks, but modifiers still a good thing I think :-)

So I can have multiple elements with disabled modifier like item_disabled and list_disabled and still can understand that it's a modifier and not an element like item-disabled or list-disabled.

@sokra
Copy link
Member

sokra commented Oct 22, 2015

ok, you are propably right.

@kumarharsh
Copy link

@sokra @nkt would it be possible to introduce yet another flag which removes the uncamelized version?
I'm asking because of a comment on one of the issues: clinyong/vscode-css-modules#3 (comment)

Basically, by retaining both the camelized and original versions of a classname, the bundle size increases, sometimes by a lot for larger projects. If all developers are in with the syntax, perhaps it'd not be as problematic to remove the original classnames? Two extra options can be added to the camelCase options: camelCase: only and camelCase: dashesOnly which cause the plugin to not output the original classes. What do you think of the idea?

@joshwiens
Copy link
Member

@sokra focuses on webpack and core integrations. @webpack-contrib/org-maintainers is what you are looking for.

A merged pull request from a year ago is not the place to be asking for an enhancement. Create a new issue so it can be properly triaged by the correct group of people.

@webpack-contrib webpack-contrib locked and limited conversation to collaborators Mar 7, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants