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

Convert v2 POC styles to CSSNext #4539

Closed
1 of 4 tasks
traviskaufman opened this issue Jul 11, 2016 · 12 comments
Closed
1 of 4 tasks

Convert v2 POC styles to CSSNext #4539

traviskaufman opened this issue Jul 11, 2016 · 12 comments

Comments

@traviskaufman
Copy link
Contributor

traviskaufman commented Jul 11, 2016

Follow-up to #4514

  • Convert all Sass files to postCSS + CSSNExt
  • Remove sass from build system
  • Update any/all docs which reference sass to postCSS
  • Update examples if necessary to not use sass
@traviskaufman traviskaufman added this to the V2 setup milestone Jul 11, 2016
@traviskaufman traviskaufman self-assigned this Jul 11, 2016
@traviskaufman
Copy link
Contributor Author

This unfortunately isn't going as well as I had hoped 😐. After working on this for the past two days, I believe we should revise our decision in #4514 on reconsider SASS as our source style language. Reasons below:

Emergent specs are less powerful and more subject to change than SCSS

Some aspects of CSS4, such as nesting and mixins, are not only less powerful than their sass equivalents (mixins don't take arguments, nesting is not should not be concatenative, doesn't work with media queries, and requires a more verbose syntax) but haven't even made it to the W3C Working Draft level (see https://www.w3.org/2005/10/Process-20051014/tr.html#RecsWD). This means they're highly likely to change, creating a lot of churn and technical debt for us.

Furthermore, other transformations are limited by static compilation, such as custom properties only working within the :root scope.

The PostCSS plugin ecosystem is young and requires infrastructural investment

Many PostCSS plugins like autoprefixer are mature, maintained, and work great. Many others, however, are immature, buggy, and/or unmaintained. This puts a lot of effort on us to very specifically track versions and invest a lot of time and energy in infrastructure rather than our own codebase.

@import will always require pre-processing

Because of our use of @import as well as bleeding-edge specifications, it is highly unlikely that we'll ever be able to serve our source css to a browser without some sort of preprocessing.

Mature, reliable tooling means more community willingness to contribute

I believe that in terms of learning curve and willingness to contribute, the community is far more likely to contribute to codebases built on mature, reliable, and robust tooling rather than bleeding-edge specifications that are constantly changing in terms of both specification and implementation. For example, even though angular2 uses Typescript, they still receive a lot of community contributions. Even though angular/material2 uses typescript + sass, they still receive a lot of community contributions.

We can still use emergent CSS features within SCSS

We can still use emergent CSS features within sass, given that scss is a strict superset of css. That means code like this works just fine.

We need the power of SCSS for the complexity of Material Design

An excellent paradigm in modern engineering is keeping tooling as simple as it possibly needs to be to build the product. While SCSS is complex and powerful, that complexity and power is warranted by Material Design. Material Design as a design system is not simple, and requires a lot of advanced styling in order to implement it in a correct, efficient, and cross-browser-compatible manner. By taking full advantage of what SCSS gives us we're able to implement it in the most robust and maintainable way, rather than have to sacrifice the readability or maintainability of our source files to align with emergent standards.

@Garbee @sgomes @rfru WDYT?

@Garbee
Copy link
Collaborator

Garbee commented Jul 15, 2016

@import is in a working standard so one day it could be shipped to browsers. However, there are some complexities with resolving paths properly, those can be addressed later though. That specifically shouldn't be a show-stopper by any means.

As far as the ecosystem, yes it is immature overall. However, nothing precludes us from using versions that are known to work for our needs and stick to them. Then, when we need something from a newer version we analyze an upgrade. We should be doing this anyways for various reasons, static versions in NPM are better than variable ones. Humans make mistakes and SemVer doesn't work.

I think trying to focus on community contributions was an improper way to try and assess the situation. This is a Google project, of course we are going to get some level of willingness from the community no matter what stack we chose. And gauging the "ease" of contribution is also infeasible since that depends upon each contributor's individual experience.

We of course can use emergent things in Sass. However, that comes with the cost of everything else Sass allows. Which means we'd need to spend more review time assessing whether X feature of Sass should be used or not to keep the code only as complex as necessary to achieve a given task. The @supports example specifically is pure wasteful. When we can pre-compile that down while browsers don't support custom vars and the just ship it by default, it is only using extra space on the network that most won't need. Once again, coming down to us being extremely judicious about what we do allow compared to it being a very explicit action when something new is being used.

The "readability" is entirely once again dependent upon what we are comfortable with as individuals. And once again, comes down to us forcing ourselves to be judicious with Sass functionality usage in order to maintain good readability.

I specifically did not want us using CSSNext for an "easy-mode" because I knew these stack problems would happen. You may require a specific version of CSSNext, but, it does not require a specific version of anything. So random breakage is bound to happen. So far, none of the problems encountered after breaking the problem down have been PostCSS's fault (aside from one issue yesterday which was extremely weird.) It was all syntax mistakes either from conversion or quickly trying to get it done. Today, the biggest issue is the way lerna is doing the internal package structuring. It is proxying the packages and not symlinking their locations. That means, import references of package names in the CSS don't work. We need to hard-code their paths or figure out something else to do instead of using Lerna.

@rfru
Copy link

rfru commented Jul 15, 2016

My top priority would be maturity and stability from the toolchain side and minimizing risk unless the benefits of moving to something new are substantial.

In regards to CSSNext, we have, in the past, worked heavily with future-forward standards on the JS side of things, and the reality is that the specs and tools have often been a moving target which has really undermined the promise of "future-proof" code. So I do think we should maintain some healthy degree of skepticism about even specs that appear to be agreed upon and uncontroversial. The lengthy timeline towards actual implementation almost always causes friction.

The benefits of PostCSS alone I think would be closely tied to the question of whether it's better to blacklist features or whitelist features. In general, for production-level concerns, I would agree the latter is usually more advantageous. In this case, for whitelisting, features == plugins which means that there will be code variance that needs to be vetted on a per-plugin basis (or at least expect non-uniform breakage). For blacklisting, we imply a somewhat more monolithic approach, in which case the variance reduces to a single project. I don't know the PostCSS community well enough to comment on the quality and track record of its plugins, but I do lean towards Sass as the default and tried-and-true option.

If I'm not mistaken, we can fairly easily enforce feature restriction in Sass with appropriate linters.

@traviskaufman
Copy link
Contributor Author

@Garbee - all valid points which I'll attempt to summarize and respond to.

First in regards to @import

lerna's package handling prevents @import paths from working correctly.

@import can be made to work by simply adding path.resolve('./packages') to the path option when requireing postcss-import within our webpack config. We did this same thing for sass. This allows us to circumvent lerna not pulling in all pkg files. I agree that this is a limitation on lerna's part but it's easily worked around.

There's a standard for @import

@import url(....) !== @import 'pkg-path';. The former is meant to be evaluated by browser at runtime, the latter is meant to bundle source files together. Multiple imports mean multiple network requests, and serve two different use cases. Furthermore, even if import is a specification, we import based on webpack's module resolution, not based on URIs.

Let's discontinue the discussion on @import for now and focus on the premises of your comment.

Premise: We can solve the postCSS immaturity problem by adhering to strict versioning. We can then analyze and decide to upgrade on a case-by-case basis, on our own time.

I believe this is an inadequate solve.

As I've previously mentioned above this leeches time and energy away from fulfilling the actual goal of this project, which is to build best-in-class material design UI components for the universal web platform.

Manual versioning maintenance also means the responsibility is solely on us to track not only bug fixes and enhancements to N plugins that we'd use, but it also means tracking the specifications themselves. Consider:

  • We decide to use postcss-apply at it's current version.
  • We chip away building components, keeping postcss-apply at the same version while its versions continue to increase
  • By the time we reach beta for v2, the apply spec introduces a breaking change / feature, which postcss-apply incorporates
  • Users of our components begin to file issues because our components are using an obsolete version of the specification and/or because they can't build against our source files with the version of postcss-apply that they're using.

In the case of sass, we either avoid this problem altogether by not using specifications until they at least become W3C Candidate Recommendation, or we make modifications to source files.

In the case of postcss, not only will we have to make modifications to source files, but we will have to begin working our way up from the version of postcss-apply that we're on to the most recent version, fixing any compatibility issues along the way. Having to do all of this regression work will slow us down from building new components as well as slow us down from addressing user bugs within the components themselves. The slowdown means that more and more issues begin to snowball. Before we've even reached beta we have a massive amount of technical debt to take on.

I'd like to prevent this scenario as much as possible.

Premise: SemVer is prone to human error and therefore suboptimal.

While I agree with the premise I'd argue that popular packages which are depended on heavily are held to the standard of proper semver. Otherwise, users would deem the package as unreliable / unusable, not depend on it, and therefore it would not be popular. I imagine authors of brand new postcss plugins that don't have many dependents and are still in v0.x are less pressured to follow semver than say node-sass. For mature, stable codebases in the JS ecosystem, semver is almost always followed and therefore optimal.

Premise: Sass can be easily abused, leading to confusing source code, poor readability, and indeterminable output.

As @rfru mentioned many of these concerns can be immediately addressed by proper use of a linter. sass-lint supports rules for limiting nesting depth, limiting BEM depth, disabling certain selectors, disabling language features, and also supports writing custom rules.

In terms of the bigger picture, I think it's important to ask ourselves: why do we want to switch from sass/scss to postcss?

Based on our discussions I believe it boils down to three reasons:

We want to be standards-compliant

  • As has been mentioned many of these standards are not yet standard 😄 which means they'll be subject to constant change and cause churn.
  • Because SCSS is a strict superset of CSS, we can be standards-compliant by using it and incrementally migrating to standards as they stabilize.

We want to eventually serve our source files directly to the browser

While I stand by that this is a lofty goal, we could always do a full migration in the future, once the dust has settled on all of these bleeding-edge specs. Waiting until specs mature to introduce these changes creates a lot less friction than wrestling with them while they're still in flux.

Sass is a big foot-gun

Linters are a big safety.

To summarize, I am not convinced that switching to PostCSS from SCSS yields any net benefit given the requirements of this project.

@Garbee
Copy link
Collaborator

Garbee commented Jul 15, 2016

So, clearly trying to not use Sass at all won't be happening. Why not go halfway and use Sass for what it is, an aid. We can still code using direct CSS custom variables in the source (without some mixin @support sorcery) and then run that through PostCSS to convert it down, as well as doing anything else we want to rely on standards for (like color() for color manipulations). That way we can use Sass for what it is good for without creating a "sass-only" source base for the things we do have alternatives for.

@traviskaufman
Copy link
Contributor Author

That SGTM. It also means as new css features become more standardized, we have the infrastructure in place to switch to them.

However we wouldn't be ruling out using things like sass variables, right? Since there's some stuff that may not be appropriate as an exposed CSS custom property (for example mdl-checkbox-mark-path-length).

@Garbee
Copy link
Collaborator

Garbee commented Jul 15, 2016

Yea, we use Sass vars where appropriate (especially when doing math and for things people shouldn't have a real reason to change) and normal CSS vars for things like colors primarily where developers do have a good need to want to change it in the future (possibly client-side as well.)

EDIT:

To clarify the usage a bit, if it is something we'd !default to make part of some public API for the styling, we should CSS Var it instead. Otherwise, keep as Sass variable since that is internal and only we need to know it. Should a point come later where it needs to be opened up, we can assess those and how to go about them on a case-by-case basis. Colors are always public vars and that is pretty much the only hard-set rule here.

@jeddy3
Copy link

jeddy3 commented Jul 16, 2016

Linters are a big safety.

It looks like you're evaluating the usefulness of stylelint. I hope you don't mind me chiming in on this issue, but over at stylelint we've thought a lot about the safety that linters can provide. Especially when used to (dis)allow certain language features.

More recently we've thought specifically about the difficulties of supporting monolithic pre-processors that use non-standard constructs e.g. operators, //-comments, variable interpolation, lists and maps, etc. David Clark, a co-creator of stylelint, recently wrote about these difficulties.

Our recent thoughts seem pertinent to the issue in hand, and so I figured you might find it helpful (in coming to a decision on this issue) if I expand on these below.

I believe that all the style linters available currently lack the ability to blacklist non-standard constructs. I think this problem hasn't been addressed because:

  • It's error-prone and difficult to target these constructs.
  • The emergence of modular processors means the problem can, in some cases, be avoided.

That's the bad news. The good news is that you can build a pretty big safety-net, whichever approach you take. It's currently possible to (dis)allow nearly all of the CSS language features and the SCSS language extensions that use standard CSS constructs (e.g. at-rules and functions) by using following stylelint rules:

Additionally, there are these rules for controlling specificity:

And these *-pattern rules for enforcing naming conventions:

As an aside, it looks like your using a BEM-based naming convention. We recommend using the stylelint-selector-bem-pattern stylelint plugin instead of the *-pattern rules above. This stylelint plugin wraps the postcss-bem-linter PostCSS plugin, and can be used to enforce strict BEM naming conventions.

As you can see, it is possible to build a hefty safety-net using the rules above, irregardless of whether you choose a CSS approach or use Sass as an aid. Sass language extensions that use standard CSS constructs can be controlled using these rules too. For example,at-rule-blacklist/at-rule-whitelist can be used to disallow @warn, @extends, @include etc. However, as I mentioned at the beginning, SCSS language extensions that use non-standard constructs can not currently be disallowed. Using a monolithic pre-processors will put a hole in that net, whereas using a modular one will not.

It might be possible to shrink that hole by trying to write a few stylelint plugins that disallow these non-standard constructs e.g. adding scss/no-interpolation, scss/no-operators, scss/no-maps, scss/no-lists etc to the stylelint-scss community plugin . However, this will involve writing error-prone heuristics, rather than relying solely on the PostCSS AST.

Alternatively, you could further explore ways of not using SCSS as an aid. Perhaps, component composition, custom properties, a bit of code duplication would be enough. I don't know enough about your intentions for this project, or the historical hurdles you've overcome by using SCSS, to comment further on this though.

There no clear right answer here. The size of the hole will depend on the number of non-standard constructs you allow. And so the benefits of using SCSS as an aid might outweigh the benefits of having no hole in your linter setup. Either way, I figured that detailing the capabilities (and limitations) of stylelint, and other style linters, will help you somewhat in your decision making.

Myself, and I suspect anyone else on the stylelint team, would be more than happy to answer any questions you have about stylelint. So please don't hesitate to holler if, for example, you run into any obstacles while configuring it :)

@traviskaufman
Copy link
Contributor Author

traviskaufman commented Jul 18, 2016

@jeddy3 thanks for taking the time to write all that up!

You are correct that we're evaluating stylelint and it's currently our top choice (thanks for pushing out v7.0.3 this morning btw!). We like it because it is extensible, seems very well maintained, and (unlike tools like sass-lint) is built not only for specific dialects but for general CSS linting as well. I'm currently working on integrating it now as part of #4464 and haven't run into any problems yet (besides the selector-class-pattern false positive which was patched). I appreciate the offer to answer questions / give guidance and will of course let you know if I run up against any walls.

In terms of scss restriction, I actually feel that the blacklisting / whitelisting rules will help us get most of the way there, especially in terms of at-rules and functions. We'll definitely be using the BEM linter as well so thanks for pointing that out to us!

traviskaufman added a commit that referenced this issue Jul 18, 2016
* Add Stylelint config
* Add BEM linting
* Add SCSS linting
* Fix all general lint errors
* Remove theme scss files as we won't be using them moving forward

Part of #4464
Relates to #4539
@jeddy3
Copy link

jeddy3 commented Jul 20, 2016

I'm currently working on integrating it now as part of #4464

Fantastic. I hope it goes well.

I actually feel that the blacklisting / whitelisting rules will help us get most of the way there

I agree. I suggest avoiding non-standard CSS constructs (operators, //-comments, variable interpolation, lists and maps) where ever possible though as these (particularly operators outside of calc functions) are difficult to lint and might be troublesome later down the line.

Well, that's all the linting advice I have. Good luck with V2 btw :)

traviskaufman added a commit that referenced this issue Jul 20, 2016
* Add Stylelint config
* Add BEM linting
* Add SCSS linting
* Fix all general lint errors
* Remove theme scss files as we won't be using them moving forward

Part of #4464
Relates to #4539
@ai
Copy link

ai commented Jul 20, 2016

@traviskaufman yeap, most of W3C specs could not replace Sass. This is why I am still fan of PreCSS. It didn’t give you full Turing complete programming language, but:

  1. It is faster, than Sass.
  2. It has mixins, vars and nested. They are very similar to Sass.
  3. JS mixins are much more powerful.
  4. Having pure PostCSS build system will be much easy. You will have only one parse step for Autoprefixer, PreCSS and Stylelint. They are all will be compatible.
  5. PreCSS is pure JS file (no problems with C++ compiling) and very simple inside.

What do you think about it?

@ai
Copy link

ai commented Jul 20, 2016

You could make CSS build process faster by replacing gulp-css-inline-images to postcss-url with image inline option. Parsing step is a longest operation and having common framework (PostCSS) beside all CSS tools is always easiest way.

Also we have very interesting postcss-inline-svg plugin. It inlines SVG and allow you to change SVG colors or other styles.

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

No branches or pull requests

5 participants