dgp1130 / rules_prerender

A Bazel rule set for prerending HTML pages.
14 stars 0 forks source link

CSS modules #37

Closed dgp1130 closed 2 years ago

dgp1130 commented 3 years ago

I'm wondering if we can rework CSS support to use a DX closer to CSS Modules. This might be a more standardized from of using CSS. Consider:

import styles from './styles.css';

export function renderComponent(): string {
    return `
        <div class="${styles.firstClass}"></div>
        <div class="${styles.secondClass}"></div>
    `;
}
.firstClass {
    color: red;
}

.secondClass {
    color: blue;
}

This would give style isolation for components and still be technically tree-shakable.

dgp1130 commented 3 years ago

I'm thinking this could work by taking the styles attribute of prerender_component(), and compiling each to .js and .d.ts files with some tool. The .d.ts file would allow this to pass type checking without the need for a special compiler plugin (not sure how the language service would work with Bazel). The generated files would be a dependency of the component's prerender ts_library(), which would make it usable during renderer invocation.

We would also need to propagate any imported files to the client CSS bundle. Simply importing the file is enough to include it, so we can have a Bazel provider which returns all the CSS modules listed as dependencies of prerender_component(). These can be included in the style entry point and bundled with app CSS. We can also tree shake unused styles by running PurgeCSS on the final page. One edge case is a prerender_component() with a CSS module dependency which is not imported. This would still persist to the CSS entry point and require PurgeCSS to drop it. The end result is the same, but there's a bit more processing than should be necessary. I think this isn't too big a deal, because you just shouldn't include that CSS module as a dependency in the first place.

The same styles could also be loaded at runtime if the app used client side rendering. We would (probably?) need a Rollup plugin to resolve imported styles and bundle them into client-side JavaScript. This might make tree shaking a little harder, since Rollup needs to resolve property usages of the imported file statically, which isn't necessary for prerender code. I think this should still be doable, so it might be worth supporting.

Another advantage of this strategy is that loading CSS uses the same semantics as standard JavaScript, no weird includeStyle() function required! This makes the script more statically analyzable and removes the need for a compiler plugin to support strict deps, since @bazel/typescript already covers strict deps for ES imports (#2, #3). Conceptually we could do something similar for client-side JavaScript. This could look like:

import myComponent from 'client:./my_component';

export function renderComponent(): string {
    return `
        <my-component></my-component>
        ${myComponent.script()}
    `;
}

Any import with a client: prefix would be wrapped like CSS modules and expose a different API. This would include a script() function which is effectively equivalent to today's includeScript(). We could arguably add other symbols here and provide stronger integration with prerendered HTML, but I'm not entirely sure how that would work at runtime. If we can make something like this work, then we can leverage ES module imports for client side JavaScript and CSS, removing the need for the compiler plugin.

Refocusing on CSS modules, the biggest issue here is standardization. I'm not too sure about the status of this proposal but some tooling seems to already exist. If the standard isn't fully nailed down, it may be a bit preemptive to do this. I should do a bit more research here.

I'm also not entirely sure of the status of a lot of tooling here. Apparently there are some Webpack, Babel, PostCSS plugins which do this, but I haven't yet found a CSS -> JS module converter which that I would actually want here. I'll need to understand a little more what tooling is currently available and how it can be plugged in to rules_prerender.

The biggest Bazel issue is the composes keyword, which allows one class to mixin another. On its own this is fine, but users might reasonably want a class from one CSS file to depend on a class from another CSS file. These users would want to express this relationship via Bazel in some kind of css_library(), which does not exist to my knowledge. Admittedly this is already an issue since one stylesheet might depend on the global styles of another, but I think composes makes this a bit more of an issue. I'd rather not make a generic CSS ruleset just for rules_prerender. Someone else should own that, but I'm not aware of any active effort on this front. (We could use the Sass rules for this, but that comes with the overhead of Sass, which some users may not want.)

dgp1130 commented 3 years ago

I took a quick pass at a prototype for this in ref/css-modules.

It actually worked out a lot better than I expected. postcss-modules provides the exact functionality I need and plugs in relatively easily with rules_postcss. It generates a .module.css file equivalent to the input except with class names obfuscated. It also emits an object with key-value pairs mapping the plain class name to the obfuscated name. I was able to use this to generate a .css.ts file and stick it in a ts_library(), then depend on it from the prerender library. This allows the prerender library to successfully import it with:

import styles from './my_styles.css';

export function renderComponent(): string {
    return `
        <div class="${styles.myClass}"></div>
    `;
}

And it output the obfuscated class name successfully! I was even able to get the IDE working with generated files by setting rootDirs and paths correctly (though it requires a build to generate the .css.ts file before the import resolves unfortunately). There are still a few things to confirm before we can move forward here.

  1. postcss_multi_binary() does not support declaring additional outputs so I had to resort to using a bunch of postcss_binary() targets. We might want to consider contributing this feature to rules_postcss. Since postcss_binary() supports it, this was likely just an oversight.
  2. Styles do not appear to be scoped. Plain selectors like div are untouched and make it to the global stylesheet. postcss-modules does seem to support scoping, but this just didn't work. It seems to add some special plugins to support this, so I may need a Bazel dependency on those packages for this to work.
  3. Currently I'm generating a TypeScript file which uses an export default { /* styles */ }. This allows us to include kebab-case class names and use them as such. We should consider using export const /* styles */ and camelCasing the name. Not currently convinced one is better than another.
  4. We should no longer need to extract styles and generate an entry point, since all styles listed in Bazel deps are assumed to be used.
  5. As a result of 4. we need to tree shake class names instead of files. We should integrate PurgeCSS at the page level which should be able to handle this.
  6. Do we allow users to import CSS modules into client side JS? Currently the CSS module ts_library() is private, so apps shouldn't (but technically can) do this. It does allow the client to the know the obfuscated names, which can be useful for some instances (document.querySelector('.' + styles.foo) for example). However it does complicate the tree shaking strategy, since the JS can create a DOM element with any class it has a reference to. When optimizing the client-side JS, we would need to track which styles were preserved, and then make sure PurgeCSS keeps those classes. Alternatively we could just say that client-side JS usage does not prevent tree shaking, which would be good enough for some usages.
dgp1130 commented 3 years ago

Actually, regarding point 6, this would break with obfuscated code, where the generated class names get converted to something smaller in HTML/CSS, but would not be updated in JS. I suppose we could search and replace such strings in JS as well, but that gets a bit sketchy and I'm not convinced it's a good idea.

I vote to punt on 6. until we have a compelling use case to justify the feature.

dgp1130 commented 3 years ago

Regarding point 1. I filed an issue and offered to add the feature: https://github.com/bazelbuild/rules_postcss/issues/64.

Regarding point 2. I think this is WAI. Looking around some of the postcss-modules-* plugins, it seems that only classes are scoped. I guess that makes a certain amount of sense, because it can't know which part of the HTML should be scoped to a particular CSS module. By convention, prerender components should have a parent class (.myPrerenderComponent for example) and all the styles in the associated CSS module should be child selectors of that class. That's more of a convention/best practice than something which can realistically be supported by tooling.

Regarding point 3. I think I'm going to stick with the current export default {} for two reasons.

  1. All the CSS module examples I can find use a default export, so this is consistent with their convention.
  2. CSS classes can include hyphens while JS symbols cannot. This means a class like foo-bar cannot be directly imported to JS with the same name.
    • postcss-modules does support camel-casing the names, but this is an awkward translation for users (not clear that import { fooBar } from './styles.css refers to .foo-bar { }) and there is the possibility of conflict (what happens if the user defines .foo-bar { } .fooBar { }?)
    • We could use named exports for all camelCased symbols and a default export of all symbols (including snake-cased class names), but this complicates the import story and hides certain classes in a non-obvious way.

I think the simplest answer here is to just use a default export.

Regarding point 4. I was able to update the prototype to generate a style entry point based on all the transitive style dependencies, regardless of their usage. This is a lot simpler than extracting usage from the generated HTML document.

Regarding point 5, I was able to make a prototype which called PurgeCSS to drop unused classes based on the HTML.

All this processing does causing issues with sourcemaps, which have been broken for a while and I haven't bothered to look into them yet. I'm not sure if PurgeCSS has sourcemap support, but we would probably skip that step for dev builds anyways. Regardless, sourcemaps are still nice in production if the project can get away with them, so it would be nice to support. Regardless that can be dealt with later and isn't necessary for an initial implementation here.

My current prototype is in ref/css-modules-2. I think the next step is to start breaking up the prototype into concrete, non-breaking changes, and start merging them. Once we have a basic implementation and I can migrate all the examples, then we can remove the old infrastructure.

I'll also need to do some experimentation with "client-side JS modules" to see if I can leverage the same import trick for client side JS modules. I'll need to see if this is better than includeScript().

dgp1130 commented 3 years ago

Started cleaning up some of this code and came across an issue I hadn't noticed. The current scheme requires direct references to each CSS file used by a prerender_component(). What this means is that a component like this will fail to build:

filegroup(
    name = "styles",
    srcs = ["foo.css", "bar.css"],
)

prerender_component(
    # ...
   styles = [":styles"],
)

The reasoning is because we need to run postcss_binary() for each and every CSS source file to run the postcss-modules plugin. However, at loading time there is no way for prerender_component() to know how many CSS files are present under :styles or how many postcss_binary() targets to create.

If we can fix https://github.com/bazelbuild/rules_postcss/issues/64, I think postcss_multi_binary() can probably handle this case which would make things a lot simpler. However, this pulls on a broader composability issue. It should be possible for one CSS file to depend on another via the composes field in the CSS module or @import. Generating the CSS module's TypeScript would also be repeated for every prerender_component() which uses a CSS file.

I'm thinking a proper toolchain around CSS modules would be really helpful for this. Something like a css_library() or css_module() rule would be able to abstract these dependencies correctly. This would allow one CSS file to properly depend and compose one another. This would also allow generating CSS module TypeScript at the library level.

I'm not aware of any existing rules which quite satisfy this niche. rules_sass and rules_postcss comes close, but don't quite fill it. There's a lot of hidden work here which comes from tooling like strict deps supporting @import and composes. I'd really rather not build this from scratch for rules_prerender, but it is frustrating that nothing else seems to exist here.

I posted on the Bazel slack to see if there is a CSS ruleset I'm not aware of that could be helpful here. If not, some of the alternative options would be:

  1. Don't support prerender_component() depending on filegroup()s for styles.
  2. Use postcss_multi_binary() to dodge the immediate problem and hope this doesn't come up again.
  3. Write css_library() / css_module() from scratch.
  4. Don't support shared styles at all.
dgp1130 commented 3 years ago

I started exploring declarative shadow DOM in #38 which I think might be an interesting alternative to address a lot of the same problems. The issue of "how do I share a common stylesheet amongst several components" is still relevant, so we may need some kind of tooling to support that. But declarative shadow DOM may be able to handle some of the CSS scoping issues that required CSS modules to solve.

dgp1130 commented 2 years ago

I landed declarative shadow DOM support a while ago and I think it is a better approach to style isolation being a web standard (or at least close to it). I'd rather not integrate CSS modules and make user's write CSS that's not real CSS and have a difficult migration as the platform evolves and CSS modules get replaced by whatever the next thing is.

Declarative shadow DOM doesn't solve all our style problems, there's still a need for css_library(), inlined styles need to be bundled, etc. But I don't see those problems being meaningfully addressed with CSS modules, and until I have a compelling problem for them, I don't think CSS modules are the right path forward.