CSS encapsulation #677
CSS encapsulation #677
Conversation
Co-authored-by: Robert Mosolgo <[email protected]>
Thanks @elia!
joelhawksley
force-pushed
the
css-encapsulation
branch
from
d8996d8
to
6bca5ad
Compare
|
This seems like a very interesting approach to solving for the use cases described. My comment is more a philosophical one: should this be part of ViewComponent at all? I can think of some reasons why VC could simply provide a few hooks in the base class(es) so that this particular CSS solution would function through a separate gem. That way (a) it's optional, and (b) others could build different solutions in a similar fashion. To be perfectly honest, I never use CSS modules (aka fingerprinting class names) nor HTML-embedded stylesheets, and prefer custom properties, custom elements, shadow DOM, etc. for encapsulation techniques…and so this isn't a solution I personally would use. |
|
This is great! I'll take some time to try it next week, but here are some preliminary thoughts after reading the PR:
|
lib/view_component/css_module.rb
Outdated
|
|
||
| # Generate a short, random-ish token to prevent CSS selector collisions. | ||
| def compute_hash | ||
| Digest::MD5.hexdigest(@module_name).first(5) # rubocop:disable GitHub/InsecureHashAlgorithm |
There was a problem hiding this comment.
What about using the first 5 characters of the Base64 representation of the hexdigest? This way there's more "specificity" in 5 chars, and even less risk of collision.
- inspired by
css-loader'slocalIdentName: "[path][name]__[local]--[hash:base64:5]"(source) - see https://stackoverflow.com/a/9987466/1789900 for an example of implementation
lib/view_component/css_module.rb
Outdated
|
|
||
| # Generate a short, random-ish token to prevent CSS selector collisions. | ||
| def compute_hash | ||
| Digest::MD5.hexdigest(@module_name).first(5) # rubocop:disable GitHub/InsecureHashAlgorithm |
There was a problem hiding this comment.
MD5 is pretty good at generating (mostly) collision free hashes, but a couple of points:
- It's counted as encryption and federal compliance regulations have specific rubrics around encryption standards. Using this for "enterprise" software involves some bureaucratic complexity. It might be easier to migrate from this just to avoid having the conversation.
- JS does not have MD5 in its standard library. It's unlikely to get it as it's now an obsolete standard. It's also a non-trivial implementation; it requires quite a lot of code to implement or pull in, so if we need to mirror these on the client side we'd be looking at adding 1-2kb of JS code just to do this.
Perhaps a better option is to use a simpler digest mechanism, such as base64? is probably more ubiquitous than MD5, and where it isn't it can be implemented in a dozen or so LOC. It does have a much higher potential for collisions; but given it's used as part of a class selector here, the risk of collisions is likely the same as using MD5.
There was a problem hiding this comment.
The problem with simple truncated base64 is that you'll have the same value if components have the same prefix -- it's only encoding the string with a different radix, not hashing it:
Base64.encode64("HelloWorld").first(5)
=> "SGVsb"
Base64.encode64("HelloThere").first(5)
=> "SGVsb"I'm not sure if there is some other hashing function that does not imply the compliance regulations you mention?
There was a problem hiding this comment.
I don't think this is necessarily a problem as the class names would still be different as @module-name is used both undigested and digested parts of the selector: a full class would be .HelloThere_SGVsb_foo or .HelloWorld_SGVsb_foo. The intent of the compute_hash function seems to be to provide enough entropy in a class name so as not to collide with human authored classes; .HelloThere_foo is more likely to already exist in existing CSS.
There was a problem hiding this comment.
So happy to finally see this released! 👏
Just a few considerations/needs I would have as a potential user of ViewComponent::Stylable.
How can we support porting existing SCSS code and running tools like postCSS and CSSLint?
Using params to store the list of compiled CSS might not play well with caching. Using a current attribute with a Set in it would probably look better, but I'm not sure it really solves the problem.
I'm under the impression that for some application would be as fine to just ship the collected CSS of all existing components. That has the advantage of leveraging the browser cache, or not even parsing the bundle more than once by using Turbo. This approach retains most of the properties/advantages as the current solution but simplifies a lot the delivery.
I'd like to see a configurable CSS scoping prefix, ideally I should be able to replicate popular solutions like turning Foo::BarBaz into .foo--bar-baz. As long as the components are stored in global constants any string derived from the class name should work.
That's all I've got for now, hope this feedback will help in shaping a solution that can be effective for apps of all sizes! ✨
PS.
My experience building two e-commerce stores with CSS and components has been great, we implemented a poor man's version of styled components by just using unique CSS class names based on the Ruby class name. In one case it was built from scratch and we only included a reset and a micro CSS framework. In the other there's plenty of legacy CSS, so far the extraction has been quite painless. The only headaches we had were around components accepting blocks which potentially have conflicting CSS class names and clashes with names coming from utility classes.
|
|
||
| We considered allowing for evaluation of Ruby inside CSS files (at compile, runtime, or both) but instead decided to lean on [CSS custom properties](https://developer.mozilla.org/en-US/docs/Web/CSS/Using_CSS_custom_properties) for injecting dynamic values. | ||
|
|
||
| ### Bundle splitting |
There was a problem hiding this comment.
It's a bit more broad since it could also support JavaScript, but is it also worth considering svelte style components? https://svelte.dev/
e.g. Instead of having a sidecar file, you would define the style tag directly in the component and we would automatically extract it.
<%# the component erb template %>
<style>
.my_class {
color: red;
}
</style>
<h1 class="my_class">Hello world!</h1>Which could result in something like (omitting the style or link tag):
<h1 class="abc123_my_class">Hello world!</h1>I don't think the two approaches are mutually exclusive, but thought it might be worth bringing up.
There was a problem hiding this comment.
I think we should eventually support "sidecar" components as well as "svelte" encapsulating components. Having both options would allow you to write little encapsulating components and large sidecar ones.
For simplicity's sake, we should only support sidecars in the beginning.
|
|
||
| ### No Webpacker | ||
|
|
||
| The proposed design avoids Webpacker and other build tools. Instead, it uses a Ruby implementation of CSS Modules built by @rmosolgo, enabling greater portability and a convention-over-configuration experience that Rails developers expect. |
There was a problem hiding this comment.
I'm curious how this will scale in Ruby compared to using an external process like webpack or some other build tool. Not a blocker since we can always find ways to optimize, but thought it was worth bringing up.
I also think we may have to revisit this if we look at JS encapsulation. Kind of related to No SCSS, I think that would be a great time to investigate something like postCSS too. 🤔
| @@ -10,7 +10,7 @@ def compiled? | |||
| CompileCache.compiled?(component_class) | |||
| end | |||
|
|
|||
| def compile(raise_errors: false) | |||
| def ensure_compiled(raise_errors: false) | |||
There was a problem hiding this comment.
Should we have a separate compile method that always compiles?
Maybe compile!?
| def test_it_doesnt_rewrite_bare_element_selectors | ||
| before_css = "span {\n background: rbga(255, 255, 255, 0.8);\n}" | ||
| after_css = "span {\n background: rbga(255, 255, 255, 0.8); }\n" | ||
| assert_rewrite("item", before_css, after_css) |
There was a problem hiding this comment.
This allows for global styles, can we make this raise to disallow escaping the component scope?
There was a problem hiding this comment.
Would it be possible to add some sort of special selector to target global styles like other frameworks do? e.g. :global(span)
lib/view_component/stylable.rb
Outdated
| self.class.styles | ||
| end | ||
| end | ||
| end |
There was a problem hiding this comment.
| end | |
| end | |
| def commenter_name | ||
| commenter.name | ||
| def call | ||
| content_tag(:div, "Hello, World!", class: styles['foo']) |
There was a problem hiding this comment.
| content_tag(:div, "Hello, World!", class: styles['foo']) | |
| content_tag(:div, "Hello, World!", class: styles.foo) |
Thoughts on using methods instead of hashes? I think API wise, methods are a little easier to use plus it gives us the ability to later "enhance" the style methods with additional functionality. e.g. Finding unused CSS in the test environment
|
|
||
| ### No SCSS | ||
|
|
||
| We're not very sure about this one, but we're playing with the idea of just using plain CSS, at least to start. |
There was a problem hiding this comment.
Am I understanding the code correctly that we are using a SASS compiler to generate and traverse the CSS AST?
There was a problem hiding this comment.
Yes it seems. I was also a bit surprised when I saw the added sass dependency.
|
|
||
| We considered allowing for evaluation of Ruby inside CSS files (at compile, runtime, or both) but instead decided to lean on [CSS custom properties](https://developer.mozilla.org/en-US/docs/Web/CSS/Using_CSS_custom_properties) for injecting dynamic values. | ||
|
|
||
| ### Bundle splitting |
There was a problem hiding this comment.
I think we should eventually support "sidecar" components as well as "svelte" encapsulating components. Having both options would allow you to write little encapsulating components and large sidecar ones.
For simplicity's sake, we should only support sidecars in the beginning.
|
|
||
| ## Consequences | ||
|
|
||
| o, avoiding several common errors and resulting in more resilient user interfaces. |
There was a problem hiding this comment.
Seems like there's text cut off here @joelhawksley?
Co-authored-by: Kristján Oddsson <[email protected]>
|
@koddsson @BlakeWilliams @jaredcwhite @Spone @elia @keithamus @srt32 thanks for taking the time to review! Should this be a part of ViewComponent at all?
I think this is a fair point, Jared. As proposed here, one would need to include the Class naming conventionsThere is a lot of subtle complexity here, and the proposed approach is ultimately quite naive. For example, it does not consider the case where a ViewComponent's CSS is updated, but a stale copy of the ViewComponent exists in a view cache. For that reason and others, I expect we'll eventually want to use a hash of the ViewComponent and its sidecar files to generate the fingerprint. API for Slim templates@Spone, thanks for bringing this up! We use ERB exclusively at GitHub, so it's easy to miss these kinds of things. Given the desire to have fingerprinted CSS selectors, could you suggest how we might implement them in a more Slim-friendly fashion? Third-party CSS@Spone, I don't think this proposed architecture would work in that case. I'd expect a consumer of a library like Flickity to write ViewComponents that wrap its behavior. But perhaps you're suggesting that there might be a way to fingerprint third-party CSS? Caching
🤦 I had a feeling that the How can we support porting existing SCSS code and running tools like postCSS and CSSLint?I think that Ship the collected CSS of all existing components@elia, you're right. For a lot of applications, this is probably the correct approach. |
|
Hey everyone!
I would say "I don't think so". Here is how autoprefixer-rails works: it ships with the prebuilt JS script containing both PostCSS core and Autoprefixer (PostCSS plugin). During assets compilation, it just calls JS runtime (via ExecJS) to run this script against a CSS text. Packing every single PostCSS plugin this way doesn't sound like a good plan. Now, let me share my take on other questions. First of all, I love the idea of using CSS Modules as well as inlining
I'm a bit worried about additional runtime dependencies (currently, However, abstractizing
Maybe, there is already the answer to the question in the thread but: why can't we rely on the component name only? E.g.,
I've been thinking of using shortcuts for that, smth like: p.regular-class$container // expands into <p class="regular-class #{styles['container']}"></p>Not sure that this is possible but I would like to experiment with this. And the last but not least.
I'm a big fan of the classic Rails way and HTML-first approach. But ignoring the fact that frontend ecosystem has greatly progressed since the Golden Age of Asset Pipeline (like, about 10 years ago). PostCSS and others changed the way we write CSS. Tailwind is getting more and more popular in the Rails community (and not everyone hates I think the statement "enabling greater portability" is over-estimated. A lot of projects won't be able to use Stylable due to the dependency on PostCSS, Autoprefixer or whatever. That brings us back to the question whether this particular implementation should be the default one or not. Maybe, we can provide a common API to work with both Ruby CSS Modules and postcss-modules? That would allow us to satisfy everyone. |
Quick try: Spone/slim#1 (requires some minor changes to the ~text Lorem ipsum
/ <div class="Styled_6c266_text">Lorem ipsum</div>
p~text.is-big LOREM IPSUM
/ <p class="Styled_6c266_text is-big">LOREM IPSUM</p> |
|
To follow up on Third-party CSS, here is an example to make it a bit more concrete. Let's consider a Currently, I see 4 possible approaches:
//= require flickity
// app/components/gallery_component/gallery_component.js
import Flickity from "flickity";
import "flickity/dist/flickity.css";
In approaches To sum up, I'm not sure we need to tackle third-party CSS encapsulation (ie. alter the selectors), at least not in the initial implementation. (@joelhawksley you mentioned fingerprinting, in the case of NPM packages, maybe the |
| if !rendered_components.include?(rendered_components_identifier) | ||
| request.params["___rendered_view_components"] << rendered_components_identifier | ||
|
|
||
| content_tag(:style, self.class._css.html_safe) |
There was a problem hiding this comment.
I saw that github.com uses the permissive content security policy style-src 'unsafe-inline' github.githubassets.com.
I suppose unsafe-inline is then required for anyone using these styleable view components because of the rendering of <style>...</style> directly into HTML.
|
I have been running CSS modules with ViewComponents for about a year now — based on this pattern: https://daniel.heath.cc/2018/07/02/css-modules-with-webpacker Based on my experience an important part of the CSS modules idea is the Recently I switched back to Sprockets and simply output reference to each components stylesheet tag along with the component. So far works great, reaping the benefits of HTTP/2. At the same time I had to implement my own CSS modules Sprockets postprocessor which I would be happy to share, along with my update of the above referenced CSS modules implementation, if anyone is interested. It would be fairly easy to extract both as gems, which could be independent of the view component gem. |
|
I have to admit a large amount of trepidation around VC recommending a "CSS Modules" pattern, which by-and-large I consider an anti-pattern. I think it's reasonable to say VC shouldn't promote any particular flavor of how to write CSS. But if it were to do so, I would advocate for a design system based on CSS Custom Properties combined with custom element names for individual components to scope styles, plus Shadow DOM where true encapsulation is required. A far better—and more authentically "modern"—pattern, IMHO, and it requires little-to-none additional tooling. It's fine if you want to disagree with me, but that's actually my point…I don't see why either flavor of CSS should somehow get "blessed" by the project. My 2c FWIW. |
|
Really cool to see this happening! Scoped CSS is a very compelling feature. I work on a few long-lived apps that have been phasing in ViewComponent, and I would love to start replacing our mess of BEM/SMCSS stylesheet partials with something like this. Reading over the proposal, here's a few points of feedback I can offer:
Our shop only recently transitioned to using webpacker for building css. Long term I'm intrigued by the idea of moving it back to a ruby-based tool. Short term my team is going to prefer a solution that hooks into our webpack setup, with an eye towards transitioning to a ruby-based solution as it matures. |
|
@existentialmutt good points!
Actually, the clever thing is that it's only inlining the style tag after the first render of the component on the page, see: https://github.com/github/view_component/blob/e0c1e7b4c092eebabea8531227a8c0dd0cf5c294/lib/view_component/styleable.rb#L62-L64 |
|
@Spone This has its major pitfalls though: Imagine a list of items, style is inlined in the first one, the first one gets removed (for example the user deletes it via UI), and … oops. |
The style tag is not in the component, but just after it, so if you remove the component root element, the style will stick around. |
|
@Spone I see, thanks! |
|
👋🏻 Hi folks! Thank you for all the feedback so far. We are continuing to experiment in this space internally, and I hope to have more to share as we try more approaches. The main concern on my mind at the moment is trying to come with an approach that just works and feels conventional in the way that most of Rails does. I recognize that this is quite a challenge when it comes to assets as there seems to be less consensus on on conventions in that area in the Rails ecosystem. |
|
I find really interesting the possibility of using CSS Modules with ViewComponents. This avoid possible conflicts with names in the global namespace. Are there any updates? Or have you found better solutions? |
|
@collimarco https://github.com/rails/tailwindcss-rails Slightly different approach, but works well with components. |
|
@collimarco My 2c: the vanilla CSS way to avoid conflicts in the global namespace is to use custom elements and selectors based off of those ( I also would recommend avoiding Tailwind. I've had nothing but problems with it across multiple projects, and it's particularly ill-suited for use in a stack-agnostic design system built atop modern web standards. Much better to reach for tools like Open Props (a fantastic set of CSS variables you can use to build your own components) or full-on web component libraries like Shoelace—either way future-proofing your efforts for decades to come. |
|
@jaredcwhite out of curiosity -- can you please be more specific about what makes Tailwind ill-suited? I have recently switched to Tailwind on a fairly large codebase and so far I am having fun. Curious to hear what issues might lie ahead. Thanks! |
|
Please, let's not hijack this issue with a Tailwind pros and cons argument 🙂 |
Please read this pull request as a proposal that can and should change.
This pull request introduces a prototype implementation of CSS encapsulation. We've been running this code in production for about a month without issue.
Wading into this area is not something we take lightly, and we recognize the significant amount of prior art and context to take into account.
I've included extensive documentation in the form of an Architectural Decision Record. I'd love to see us use that document as the grounds of debate for this proposal.
I'd love to hear your thoughts at all abstraction levels, from the technical details to the broad concepts.