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

MDC Web is based on Sass, yet Sass is not well supported in React tools #109

Closed
jimbojetlag opened this issue Jun 21, 2018 · 14 comments
Closed
Assignees

Comments

@jimbojetlag
Copy link

Material-components-web is being ported to different libraries, including Vue, and it seems any library or framework other than React has been happily integrated with Sass.

It is known that the state of styles in React is a mess. This mess is two fold: First whether to write css in javascript. Seconds whether to process css by tools like Sass.

Another problem is starting a new project with React is hard. Hence, many of us opt to use CRA so that we can delegate dealing with nightmares like Webpack to other developers, which leads us to another problem: Weak or nonexisting support of Sass in CRA.

There are half baked approach for how to have Sass processing in CRA 1.x. None of them, including the one suggested in material-components-web-react guide, are ideal.

Sass is supposed to be supported in CRA 2.x, but it only works properly for the application code, and not for the npm packages that import sass files from other npm packages, which is the case of material-components-web-react components. Specifically see facebook/create-react-app#4651, facebook/create-react-app#4195 and facebook/create-react-app#4195 (comment).

Unfortunately, CRA authors are highly opinionated, inflexible and not very responsive when it comes to these Sass issues. At the moment, using material-components-web-react with CRA 1.x is possible with some hacks, and completely broken in CRA 2.x. Even Google project material-components-web-catalog uses an ejected version of CRA 1.x.

Given these high barriers, and given that Sass is a hard dependency of developing production quality with Material-components-web, the question is what can be done to stay away from Sass mess in React tools? Specifically, is it possible to change sass imports in material-components-web-react so that it can work with CRA 2.x, without CRA code being changes?

@jimbojetlag
Copy link
Author

@moog16 Would it be possible to prefix the sass import path of external dependencies with a tilde ~? That might help us to not depend on CRA for resolving this.

@moog16
Copy link
Contributor

moog16 commented Jun 21, 2018

@jimbojetlag How would adding the ~ help? You would still need some type of sass importing tool to use those files, correct? Also I believe if the Sass dependencies don't have ~s then we will still see that same issue.

RE MDC React Guide:
I understand that modifying the package.json is not ideal, but it is fairly straightforward. And once setup it works. What other issues do you run into with this process?

@moog16
Copy link
Contributor

moog16 commented Jun 21, 2018

I was also going to suggest using a different boilerplate other than CRA. There are a lot of Webpack/Sass lovers out there that have created these seed projects. It may not be as heavily supported as CRA (and not Facebook approved), but I'm sure there are some pretty solid ones. I was going to explore other options and link them in MDC React's docs. How does that sound?

@jimbojetlag
Copy link
Author

@moog16 You are right about Sass dependencies as demonstrated in the below:

Currently with CRA 2.0, importing an MDC React Sass module like this:

// App.css
@import '~@material/react-top-app-bar/index.scss';

, gives this error:

Failed to compile.

./src/App.scss)
Module build failed:
@import "@material/top-app-bar/mdc-top-app-bar";
^
      File to import not found or unreadable: @material/top-app-bar/mdc-top-app-bar.
      in <...>/node_modules/@material/react-top-app-bar/index.scss (line 1, column 1)

If we add ~ prefix in the MDC React module:

// @material/react-top-app-bar/index.scss
//@import "@material/top-app-bar/mdc-top-app-bar";
@import "~@material/top-app-bar/mdc-top-app-bar";

, then the error gets propagated to the dependency, in this case an MDC Web module:

./src/App.scss)
Module build failed:
@import "@material/top-app-bar/mdc-top-app-bar";
^
      File to import not found or unreadable: @material/top-app-bar/mdc-top-app-bar.
      in <...>/node_modules/@material/react-top-app-bar/index.scss (line 1, column 1)

I cannot see a way for MDC React to stop this error propagation.

RE MDC React Guide:
I understand that modifying the package.json is not ideal, but it is fairly straightforward. And once setup it works. What other issues do you run into with this process?

The guide is based on CRA 1.x, I was trying to get MDC React work with CRA 2.x, which claims to support Sass and sooner or later will replace the current version, making the guide obsolete. I understand that CRA 2.x is still in alpha.

It may not be as heavily supported as CRA (and not Facebook approved), but I'm sure there are some pretty solid ones. I was going to explore other options and link them in MDC React's docs. How does that sound?

The difference between CRA and other boilerplates, apart from being official, is that CRA is not a boilerplate. Boilerplates are like ejected CRA, that is you as the developer will inherit the future maintenance burden of the build tools. Depending on the project size, this maintenance may not be trivial.

I think the getting stared guide should continue to be based on CRA, but we need to push CRA to have Sass support for dependencies. Like I explained at the beginning of this issue, Sass is not a first class styling tool in React and it community, yet it is the case for MDC. This may cause MDC React having a hard time.

@moog16
Copy link
Contributor

moog16 commented Jun 25, 2018

I agree integrating Sass with CRA has been a big hurdle for us to overcome, but we've been able to get it working between the catalog and using the getting started guide. I think its too early to be looking at CRA 2.0 since its only in alpha. As you know, a lot can change while in alpha. I do see that CRA is stating that they will support Sass compilation, but to what extent and how exactly it'll be enabled is still up in the air.

+1 to being different from other boilerplates as it removes "webpack hell" and maintenance.

@mswezey23
Copy link

Noting that following the getting started guide on Windows 10 leads to this sass issue as described here. I've seen half a dozen solutions, but for someone that's not an expert at webpack configurations - it only leads to confusion. Going try out https://github.com/jamesmfriedman/rmwc to see if that gets me where I want to be.

File to import not found or unreadable: @material/react-button/index.

@moog16
Copy link
Contributor

moog16 commented Jul 10, 2018

@mswezey23 I just tried CRA with @material/react-button on windows 10 and it worked fine.

Let me know what issues you're running into and I can help :)

@nickmccurdy
Copy link
Contributor

nickmccurdy commented Jul 11, 2018

I appreciate the amount of work and technological decisions that have gone into the existing Sass styles for Material Components Web and this project, but I agree with @jimbojetlag and I'd really like to be able to customize its styles without Sass before switching from other frameworks. As far as Sass goes, in my opinion it's better to replace it with more modern CSS preprocessors like PostCSS or CSS in JS solutions like styled components for all practical purposes (especially if you're using a dynamic frontend view framework like React).

I understand that MDC may have a hard requirement on using Sass for design purposes, but I think we can drop the requirement of end users compiling and writing Sass without impacting the architecture and workflow of the existing styles, allowing users more of a choice in tooling and interop with existing React styling approaches. For example, Bootstrap 4 is written in Sass and provides CSS custom properties for customization without a build tool, which I think is the simplest and most flexible solution as long as your environment supports CSS custom properties.

@moog16
Copy link
Contributor

moog16 commented Jul 11, 2018

I am a fan of CSS in JS and PostCss. We do have PostCSS in our build process, and am not sure how we could use PostCSS to make things easier for the end developer.

I haven't used much CSS in JS in projects. How would something like that look like? And how would you go about selecting a framework. Just based off of this list there are a ton of libraries out there.

Would you be willing to do a little bit of code in one of the components to show us?

@nickmccurdy
Copy link
Contributor

nickmccurdy commented Jul 11, 2018

Personally if I maintained a project in Sass and I didn't need dynamic styles, I would port it to modern CSS (mainly custom properties), optionally using postcss if necessary for legacy browser support. I understand the team may want to continue using Sass, but you can still create CSS custom properties in Sass so they can be changed by end users that don't use Sass (see Bootstrap 4). Then a plain old CSS build can be distributed with the package, which would solve the issue of this not working with CRA without having to wait for v2 to stabilize.

I'm pretty new to CSS in JS to be honest, but I don't think it would be possible to use it in core unless you want to completely rewrite and rearchitect all styles in MCW and MCWR. That being said, Styled Components looks like the strongest option to me, but it only works with React. Emotion has a very similar syntax that's almost completely compatible with Styled Components, but it works in any JS framework. Both support writing Sass, but I would still prefer writing CSS custom properties if I were using a CSS in JS library.

Ultimately I think just adding CSS custom properties to Sass is the best approach, since they can be set in CSS, CSS modules, Sass, Less, CSS in JS, etc. As far as prior art goes, it's worth mentioning that Material UI used to use inline styles and decided to replace it with JSS after running into scaling and flexibility issues, but I don't like either approach compared to Styled Components or plain CSS personally.

@kfranqueiro
Copy link

kfranqueiro commented Jul 11, 2018

We already use CSS custom properties in a few of MDC Web's packages. However, they are not a silver bullet. Here are a few problems we've already encountered in MDC Web:

  • IE 11 doesn't support CSS custom properties. Handling variables at the preprocessor level allows us to expose customizations that can work in all browsers MDC Web supports.
  • There are things we would like to use custom properties for, but can't due to current limitations in CSS. One prime example is maintaining colors and alpha values separately and composing them together, which can be done in Sass, but has no widely-supported CSS equivalent.
  • Using a preprocessor allows us to perform calculations (some of which have no CSS equivalent) based on defined variables. Thus we can expose Sass variables that are configured pre-calculation rather than making users calculate the correct value themselves; this would not be possible with CSS custom properties exposed through the built CSS.

Also, tangentially, I'm not really sure what warrants the insinuation that Sass isn't modern. It's being actively worked on full-time. As far as I can tell, PostCSS plugins would (ideally) just be a different means to the same end - we'd still effectively need to use it as a preprocessor.

@nickmccurdy
Copy link
Contributor

I think it would really help the non-Sass development workflow if we added as many custom properties as possible, allowing users to use Sass in more advanced use cases.

  • I'm aware that IE doesn't have custom properties, but often it's more useful to have basic static custom properties than fully dynamic custom properties, so I think polyfills solve the issue well (Sass doesn't have dynamic variable at all, so it's not any worse than Sass's support given the right preprocessor).
  • That color and alpha problem sounds interesting, is it solvable with math or any other new specs?
  • I'm not sure I understand, couldn't you make a custom property based on a calculation from another custom property? Is this only an issue for calculations that can't be made in standard CSS (in which case a user can reimplement it in JS to avoid using Sass)?
  • To be honest, I don't like Sass, I think it encourages antipatterns like deeply nested selectors and specificity hacks. That being said I'm mainly against using it in React projects because it has too many features that break standard CSS enough to make extension via CSS in JS difficult. I'm fine with allowing users that like Sass to continue using it, but it causes issues with common React styling practices.

@moog16 moog16 self-assigned this Jul 12, 2018
@lynnmercier
Copy link
Contributor

lynnmercier commented Jul 18, 2018

Hey everyone, there is clearly a lot of discussion that needs to be had over this problem. I don't think a GitHub issue is the right place to talk, because I keep looking at GitHub issues and thinking "How do I close them all!" But actually, this thread is a discussion, and not something that needs to be closed.

So I propose the following: please use our "material-theming" channel (in the web category) in our Discord server. You can chat with us directly by following this link: https://discordapp.com/invite/material-components

I'm going to close this issue now. So all future discussion about Sass, CSS-In-JS, custom properties, etc can happen in that channel.

Also, as we have discussions, I will start filing more specific GitHub issues about tasks my team is working on. For example, I'll probably file a "Build a prototype using CSS-In-JS" issue soon, to show that we're looking into that specific technical detail.

@jimbojetlag
Copy link
Author

@lynnjepsen please do not direct discussions to a chat channel. There are three good reasons for this:

  1. Whatever happens in those channels, are not searchable and googlable by others. Many people who have contributed to this issue, for instance, would have never been able to contribute and make suggestions.
  2. There is no structure in those chat channels, there are no such things as threads and states to track the progress of a discussion. Those texts are just ephemeral.
  3. Your team on Discord is not responsive. Many questions are never answered. My questions and concerns in this very issues would have had a single line respond, if any. Compare that with the lengthy discussions that happened here.

You could instead simply tag these issues as discussion type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants