Skip to content

Write a lint rule against CSS modules #98

Closed

Description

@gaearon

As much as we appreciate the pain points CSS Modules are solving, we’ve made a decision not to use them in this project. You can see some discussion on this in #90.

However Webpack currently lets you use custom :local syntax to “enable” them in regular CSS files. We don’t want people to do this because this limits our ability to add more CSS tools or migrate from Webpack to other bundlers in the future.

This is why we should enforce that people don’t use :local syntax. Otherwise we’ll just randomly break their apps when we change something in how we handle CSS. This won’t be fun, and people will blame us for that even though we explicitly do not support this feature.

Luckily, it is easy to fix. We should write a custom lint rule with ERROR level that is triggered on a statement like this:

import styles from './styles.css';

This, however, should work fine:

import './styles.css';

This semi-enforces that you don’t use CSS Modules because we just don’t support them. We can ensure your CSS stays reasonably portable in case we want to change the underlying architecture.

Reference on writing ESLint rules: http://eslint.org/docs/developer-guide/working-with-rules.

If you want to work on this, please write here so we don’t have multiple people doing it at the same time.

Activity

  1. casesandberg commented on Jul 22, 2016

    @casesandberg

    I got this 🐈

  2. KyleAMathews commented on Jul 22, 2016

    @KyleAMathews
    Contributor

    In Gatsby we added CSS Modules support only for css files with names *.module.css per @andreypopp's suggestion which seems safe enough. I don't have good enough pulse on where CSS Modules are in adoption life cycle but this might be a good compromise as CSS Modules are one of the most useful things to come along in the CSS world in a long time.

  3. hzoo commented on Jul 22, 2016

    @hzoo

    @casesandberg I would use astexplorer to check the ast of the 2 forms

  4. gaearon commented on Jul 22, 2016

    @gaearon
    ContributorAuthor

    We can reconsider this in a few months. But it's good to warn until/unless we commit to supporting them.

  5. casesandberg commented on Jul 22, 2016

    @casesandberg

    @gaearon Should this be in a eslint-plugin-create-react-app module or just as a rule in config/rules?

  6. gaearon commented on Jul 22, 2016

    @gaearon
    ContributorAuthor

    I'm not sure how to structure rules in eslint so whatever is simpler ;-)

  7. casesandberg commented on Jul 22, 2016

    @casesandberg

    I'm not really sure either-- if there is a possibility we will have more of these in the future we should probably bundle them all together in one plugin.

  8. gaearon commented on Jul 22, 2016

    @gaearon
    ContributorAuthor

    At a later point, we sure can. For now let's keep it as simple as possible.

  9. jaredpalmer commented on Jul 22, 2016

    @jaredpalmer
    Contributor

    @gaearon is the plan to write linters against Aphrodite and Radium too? Or is CSS-in-JS okay to use?

  10. mxstbr commented on Jul 23, 2016

    @mxstbr
    Contributor

    Both Aphrodite and Radium are fine, since they don't use any webpack-specific features, which is what we want to avoid.

    If we ever switch away from webpack and people are using CSS modules they won't have any chance to update because it won't work!

  11. giuseppeg commented on Jul 23, 2016

    @giuseppeg

    CSJS is a good alternative to CSS Modules although I like the fact that you are not trying to enforce any CSS [in JS] methodology 🙏

  12. geelen commented on Jul 27, 2016

    @geelen

    I actually suggest a different approach — removing :local from css-loader altogether. I've started a discussion (webpack/css-loader#308) over there but I don't expect it to be resolved fast enough for the needs of this project, so I think this linting rule is a good solution in the interim.

    Interestingly, the whole concept of importing something from a CSS file is what I was hoping to achieve by proposing ICSS as a standard format — a minimal extension of CSS that allowed passing symbols around. This is what css-loader actually supports, everything related to :local is actually just a series of PostCSS transforms compiling to ICSS. And so the concept of import styles from "./styles.css" was always intended to be more widely-applicable — not specific to Webpack, CSS Modules, or anything. It just turned out to only be really used by CSS Modules.

    So I'd love to think that any future bundler of CSS would permit some form of exporting from CSS to JS and so this kind of syntax will become more standard, but I admit that's well and truly beyond the scope of this issue :)

    For what it's worth, if this project did decide to "support" CSS Modules, I think the best way would be if all files ending in *.module.css were loaded with CSS Modules like Gatsby does — I think that's the closest thing we could get to having both no configuration and totally explicit behaviour. But I get that it's not on the roadmap for the moment.

  13. jaredpalmer commented on Jul 27, 2016

    @jaredpalmer
    Contributor

    @mxstbr just wanted to doublecheck

  14. gaearon commented on Sep 3, 2016

    @gaearon
    ContributorAuthor

    Meh, probably not worth it.

  15. thien-do commented on Sep 20, 2016

    @thien-do
    Contributor

    Hi, I don't know if anyone care about this anymore, but today I saw one of my teammate use CSS Modules in CRA without :local by this way:

    import styles from '!style!css?modules!./background.css';

    is this something "no configuration and explicit behaviour" (an alternative to the .module.css approach)?

  16. outdooricon commented on Sep 20, 2016

    @outdooricon

    Yes, CSS Modules now exposes locally by default and globally by explicit declaration (:global)

  17. gaearon commented on Sep 24, 2016

    @gaearon
    ContributorAuthor

    today I saw one of my teammate use CSS Modules in CRA without :local by this way

    Please tell them it is entirely unsupported and we reserve the right to break this in any release.

    We don’t officially support Webpack loader syntax in import statements. If you can't find something in CRA docs then it's not supported.

  18. jfmengels commented on Oct 14, 2016

    @jfmengels

    @gaearon There is an eslint-plugin-import rule forbidding Webpack syntax in the path (it basically warns if there is a ! in the path).

    I have noticed you disabled the whole plugin because of its compatibility with eslint-loader, but you could probably still use rules like these, that do not attempt to load/read other files. (I'm happy to make you a list of rules that should be fine and useful if you want)

  19. locked and limited conversation to collaborators on Jan 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions