Skip to content

Instantly share code, notes, and snippets.

@ef4
Last active February 27, 2023 15:21
Show Gist options
  • Save ef4/d5347562b433b251f32b004315fd3c70 to your computer and use it in GitHub Desktop.
Save ef4/d5347562b433b251f32b004315fd3c70 to your computer and use it in GitHub Desktop.

Feedback on scoped CSS design

  1. I would expect this to be cleaner to implement in a way that works in both classic and embroider if we first do embroider-build/ember-auto-import#565 and I'm curious how you avoid needing that. I'm saying this because I'm hoping your implementation ultimately emits imports for CSS, ultimately feeding the CSS into whatever prevailing CSS pipeline exists without making assumptions about that pipeline.

  2. Does your implementation allow the CSS to split alongside the Javascript, if people use either import() or Embroider's splitAtRoutes? (This is another way to ask the same question as above, because if the styles are getting emitted within the module graph it will Just Work and if they're not, it's very much not OK.)

  3. I would strongly suggest against using classes to do the scoping. Adding dedicated data attributes is better because you can leave the user's classes entirely alone. Instead of changing the selector .foo to .foo-123, you can change it to .foo[data-scopedcss=123]. h1 becomes h1[data-scopedcss=456]. .main > span becomes .main > span[data-scopedcss=789]. This is how Vue does CSS scoping and it works out great. It also results in a very simple AST transform on the template side -- you can add the data-scopedcss attribute to every HTML element in a given template, and that keeps your scoping perfectly aligned with the boundaries of the template, even when people use arbitrarily complicated selectors.

    (Globally namespaced CSS features like keyframes do still need mangling and I agree with that part.)

  4. If you instead put <style></style> inside the <template></template> tags in GJS, you get

    • out-of-the-box correct syntax highlighting
    • scoping per-component instead of per-file. Scoping per-file should be a total non-starter given the way Ember is trying to stop forcing components and files to be one-to-one.
    • a single interface that works for both GJS and traditional HBS files (you can put <style> in those too, and thus should not even bother implementing standalone co-located CSS files).
    • An easy place in the AST to grab the unprocessed CSS, with no extra parsing step to find it.
@candunaj
Copy link

candunaj commented Feb 15, 2023

  1. I wanted to use
  • a webpack loader for hbs files
    • it will emit css file if it is part of the template <style></style>
    • it will rewrite the template
  • a webpack loader for js and gjs files
    • it will emit css file/s if there are <style> tags inside templates
    • it will rewrite templates in gjs files
    • it will add import of emited files or co-located files
  • a webpack loader for CSS files will rewrite CSS selectors in emited or colocated files

It would be amazing if ef4/ember-auto-import#565 will be done and we will be able to have one implementation for everithing.

  1. For the production build, I wanted to import styles in js components, and I tested that styleLoaders and stylePlugins will create CSS bundle automatically and bundle only used components. I am not sure how route splitting works, so I don't know about that.

  2. There are multiple approaches to the problem of scoping CSS, and we can discuss them, or even implement multiple.

  • svelte approach is to add classes p > .my-class will become p.generated >. my-class.generated
  • vue approach is to add attributes p > .my-class will become p[generated] >. my-class[generated]
  • there is another approach to rename classes so p > .my-class will become p.generated>.my-class_generated

In first two approaches styles can't leak out, but can leak into components from global styles. In the last approach, styles can't leak out or into the component. This is why I have implemented it.

  1. Is an interesting Idea. I like that. It would work for .hbs files and for .gjs files even with multiple components in it. If other would agree, then I would love to implement it.

@ef4
Copy link
Author

ef4 commented Feb 15, 2023

a webpack loader for hbs files

I don't think this should be necessary given that there's already a loader that converts all HBS files to JS files. You only need to handle the template-inside-JS case because the standalone HBS case is implemented as a trivial conversion to JS.

It would also probably imply parsing the template a second time, and the glimmer template parser is relatively expensive.

I am not sure how route splitting works, so I don't know about that.

As long as you're emitting imports of styles inside the relevant component modules as you described, it will work too.

In the last approach, styles can't leak out or into the component. This is why I have implemented it.

Well, some styles can't leak in, but there are always selectors that can leak in. I don't personally see it as a downside that a global class name works the normal web-standards way and affects everything globally. It means, for example, that if an app has a mix of tailwind and scoped CSS everything will continue to function while you're working through that transition.

Have you had a chance to explore all the dynamic class cases? Classes are not guaranteed to be present at built time at all. You'll probably be forced to create API similar to ember-component-css's this.styleNamespace, whereas with either the Svelte or Vue approaches you don't need that API. IMO because classes are a runtime feature visible to the user's code, rewriting them is fundamentally leaky.

@candunaj
Copy link

candunaj commented Feb 15, 2023

Can live reload work when I import a CSS file in the corresponding component? I was playing with that, and it seems that the application was refreshed when I changed the CSS file. Is there something I can do for live reload to work with imported CSS files?

@jacojoubert
Copy link

  1. I would strongly suggest against using classes to do the scoping.

When I first wrote the spec I took the svelte approach which is adding a dedicated scoping class. I still think that is the preferred approach. I do agree with mainmatter though that in terms of converting from css-modules to this new system rewriting the class names makes it easier. The goal I stated at the time was to add a setting to let the user switch over to use a scoping class when they were ready. AuditBoard certainly wants to get to a point where class names are left alone and only rules are mutated.

There are some concerns around [data-scopedcss=456]. Namely, when you invoke a component <SomeComponent class="foo" /> where that component has something like <div class="bar" ...attributes> you will get the external data attribute overriding the inner one. Thus, you need to go to a [data-abc123] data attribute and I personally find it noisy. I much prefer the scoped class name.

  1. If you instead put <style></style> inside the tags in GJS, you get

I think my main problem with this is that it clashes with how the web works. If I want a <style> tag to actually exist now I can't do it without us introducing some mechanism to have the processor ignore it. Putting it at a file level also solves one of the other issues we sometimes run into when building really complex layouts and components: parents needing to pass classes down.

Yes, you can use ...attributes, but often you need to pass down multiple classes to a child that needs to go on different elements. This solves that need without the introduction of a helper to convert a class name into an argument. These components are inherently tightly couples so it makes sense you would put them into the same file always. In my mind moving to a style tag inside the template tag would be a regression in functionality, though the other points you make certainly makes it appealing.

@ef4
Copy link
Author

ef4 commented Feb 27, 2023

I think my main problem with this is that it clashes with how the web works. If I want a <style> tag to actually exist now I can't do it without us introducing some mechanism to have the processor ignore it.

We already differ from the spec. For example, the spec says <Div> is a regular element but Glimmer says it's a component invocation because it's capitalized. You could say the same thing here with <Style> instead of <style> and you haven't differed from the spec in any way that's new from what glimmer already does. Personally I think that's unnecessarily subtle and <style scoped> vs <style> would be clearer to readers. (Especially since the capitalization rule will eventually be less important to learn as people adopt strict mode templates.)

I think you're underestimating how big a pain it will be to make <style>-in-Javascript play nicely with glint, eslint, typescript, prettier. It already syntax highlights correctly if you put it inside <template> instead (as long as you've already setup GJS support). As for:

parents needing to pass classes down.

This is only a problem when you're mangling the user's classes. If you leave them alone, users can pass them down normally. If they want to associate styles with those classes they can either (1) use a global stylesheet like app/styles/app.css instead of a scoped one inside a component or (2) we can explicitly support both <style> and <style scoped>.

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