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

CSS Modules local class names for Create React App #3549

Closed
wants to merge 2 commits into from

Conversation

haraldrudell
Copy link

@haraldrudell haraldrudell commented Dec 5, 2017

I though it would be pedagogical to use CSS Modules (locally scoped selectors), in a light version where the non-standard local: directive from the webpack plugin css-loader is being used. If people start making components from Create React App, with this they can’t miss how to do locally scoped css.

The point is that one way or another, we will need this and it is not going to be standard

  • Even if we do have .module.css files in 2.0, we will need a non-standard directive similar to :local and :global
  • We will need some non-standard directives should we use webpack or not
  • Otherwise local and global styles have to be in separate files
  • It does not seem w3c is going to help with standardization at the moment. The path there seems to be :scope which requires style elements in the middle of markup, eg. at the top of a component, regions or :host. You would hope they took a look at ECMAScript Symbols and found a simpler approach
  • besides the :scope aspect, it can also be foreseen that css is going to look even more like css in the future (ie. no scss cssinjs fancy-framework-dying-off-like-flies…)
  • Styling React components is not going to be standard. Either some clever mapping of possibly standards-compliant css values over to ECMAScript like css-loader, or a custom react-css-loader.

This was obviously made to work from the extensive work from the last 7 months in #2278 #2285 by @ro-savage slated for version 2.0 at some undetermined future time

harsh words on this topic are in #90

This here is best evidence at the moment

@ro-savage
Copy link
Contributor

I kind of need an ELI5 for you reasoning here. But I'll do my best

  1. With the current css modules PR Add support for CSS Modules with explicit filename - [name].module.css #2285. Standard CSS (that is global css) is imported as per usual, the same way it currently done in Create-React-App. Adding scoped css is entirely optional and only happens when you use import style from 'mystyles.module.css' and name the file [something].module.css. I can't see any reason to use :local and :global.

  2. While our config is reliant on webpack, so is the majority of the CRA config. It will all need to be rewritten if we move to another bundler/module loader.

  3. css modules is non-standard and as far as I am aware there a no plans to change that.

  4. Until something is released by w3c or someone else. No point trying to predict the future in CRA.

  5. CRA already supports some css-in-js out of the box, if someone chooses to use that. Adding CSS Modules is simply because there are a large percentage of people who use cssmodules and currently they aren't able to use CRA out of the box.

For those reasons, I don't really understand the purpose of this PR. But as I said, I didn't really understand your initial reasoning and may be totally wrong.

@haraldrudell
Copy link
Author

The purpose of this PR is to use proper component styling now.

To demonstrate CSS Modules ahead of Create React App version 2.0

@haraldrudell
Copy link
Author

haraldrudell commented Dec 16, 2017

This is also right for those who want sub-second interactive on an infinitely complex site by server-side rendering inlined css into the html and async all scripts.

Yes, React can

And the use of data attributes and bracket selectors for per-element styling

@Timer
Copy link
Contributor

Timer commented Jan 17, 2018

Closing in favor of #2285

@Timer Timer closed this Jan 17, 2018
@lock lock bot locked and limited conversation to collaborators Jan 20, 2019
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants