React-responsive-carousel: Should we adopt css-modules or styled-components?

[leandrowd]

Some of the bugs raised recently were related to CSS leaking from the application into the carousel, causing the layout to have some inconsistencies. There has been a bunch of improvements to the way we can use CSS and js together to solve these problems. All of them are solving this problem by creating some sort of scope to the classes <-> elements, making sure they don't leak.

2 of the most popular solutions are css-modules and styled-components.

Please, use this issue to voice your opinion in regards to both of them so we can decide which one to adopt.

I don't have time to work on this right now so there is no deadline for this decision yet. We will discuss until getting to a consensus and finding someone (or I find the time) to work on this.

[leandrowd]

No activity on this issue so far. I'm incline to go for css-modules as styled-components add a lot of extra page-weight and I've seen feedback where it is also causing performance problems in some applications.

[leandrowd]

After some thought, I've decided to abandon this decision. Css-modules would create a hard dependency on webpack and I believe it's important to be able to publish this component without coupling it to a bundler or other dependencies that would exclude part of the audience. It should be up to the consumer to use css in the way they prefer and it's pretty trivial to use the css from this module with both options presented here if and when the consumer wants.

[matteocng]

Hello @leandrowd sorry for replying to a closed issue, I agree that we should not create an hard dependency on webpack or styled-components, but I think we may consider the way reactstrap added simple support for CSS modules (1, 2). That way, we would just perform a className -> styleName mapping if the user passed an optional cssModule object; no additional dependencies or coupling. It would be left up to the user to configure the module bundler (in case it was required) if such user passed cssModule. What do you think?

[leandrowd]

Hi @matteocng , sounds like a good suggestion but feels weird having to mix tooling concepts with UI Params. I would be happy to be proven wrong on this. Would you like to submit a proof of concept for discussion? Then we can go for a PR if we agree on the details...

[matteocng]

@leandrowd ok I'll submit some code.

[matteocng]

I have pushed some quick code, using JS mocks for CSS modules in the tests to avoid installing and configuring additional packages (i.e css-modules-require-hook).

[leandrowd]

Hey @matteocng, your solution is creative but I think it would add unnecessary complexity to the project and I also don't like having to pass cssModule prop around.

Thinking about it I believe css modules would be a valuable addition. If we decide to add it, we should do it using what is the common/recommended practice around the community.

I believe the majority of the consumers of this component will be using webpack or browserify so might be ok to create this dependency.

Let me know what you think. If you are ok with it and would like to make this change, I would happily accept.

[matteocng]

Hey @leandrowd for now I have pushed a change to avoid passing cssModule around too much; I also think the use of compose ... global inside the .module.css file may be removed and moved to the advanced documentation since it is indeed a little bit complex, mixing locally scoped and global css. The .module.css could simply be modified to contain all expected classNames and styles required by the Carousel (or, show how to configure Webpack to use compose with the Carousel css file), assuming most CSS modules users would not want to rely on global css anymore, even just partially.

I think the strength of the "classnames mapping" method is that a) the Carousel only has to receive a cssModule object in standard format b) the user can write the css module file in whatever language (sass, less, css, postcss ...), using compose or not using it, as long as the module bundler can produce a standard CSS module object from it. In the future the module may export PostCSS functions / SASS mixins to let the user generate their Carousel CSS quickly (and avoid copy-paste and compose). c) the user can name the CSS module file freely, I suggested .module.css because it seems the community is adopting it css-modules/css-modules/issues/229.

I would be glad to apply a different/easier approach to achieve the "support non-global CSS" goal, but even creating a dependency on Webpack, I'd need an example/suggestion to take inspiration from, since module authors have found different solutions - some rely on the user wrapping the component in a ThemeProvider component with a "CSS in JS" theme, some rely heavily on PostCSS and Webpack loaders example, another example EDIT: another option .... Anyway, if a suitable solution can't be found now, we can always revisit this issue in the future in case a new method comes up.