zanaptak / TypedCssClasses

A CSS class type provider for F# web development. Bring external stylesheet classes into your F# code as design-time discoverable compiler-verified properties.
MIT License
166 stars 8 forks source link

Support CSS modules #11

Closed alfonsogarciacaro closed 3 years ago

alfonsogarciacaro commented 3 years ago

Add support for CSS modules in combination with bundlers like Webpack. When using a relative path that ends with ".module.css", instead of erasing to strings the class properties will turn into Fable import expressions. These imports will be used by Webpack that will recognize the CSS module (because of the ".module.css" suffix) and mangle the class names as can be seen in the image below, achieving the effect of locally-scoped CSS.

image

Cons: This is Fable-specific so the function will only be available in Fable projects.

zanaptak commented 3 years ago

Cool, thank you! Give me a few more days to take a look, I want to be sure I understand how it's working before pulling in.

alfonsogarciacaro commented 3 years ago

Sure! Please let me know if you've any question. TBH, I'm also concerned that the path can change the behavior of the tool to make it only Fable-compatible but I was assuming the .module.css suffix is specific enough and won't happen in other cases. But if you prefer we can make the CSS modules explicit with another parameter. Also, for this to work properly, the path must be relative to the current file and use the / separator but I'm not making any validation for now (besides checking it starts with .).

You can read more about CSS modules at https://github.com/css-modules/css-modules

BTW, thanks again for this great tool!

zanaptak commented 3 years ago

I'm ok with a Fable-specific subfeature if it doesn't add a hard dependency and the base functionality still works as before without it. Seems to be fine from my testing.

I think I prefer a new parameter for this, to make it explicitly opt-in for users, give hint that it's Fable-dependent, and remove need for specific filename format. Maybe fableCssModule=true or something like that?

I noticed that classes can be marked :global to exclude them from transformation, but then they aren't available in the internal mapping object. E.g. if we have CSS :global .clickable { ... } then clickable remains a valid class name and style$002Emodule.clickable fails to resolve. But the TP of course still includes all original class names regardless of transformation. Would it be possible/reasonable to inject the expression as (style$002Emodule.clickable || "clickable"), or maybe there is some other way to get access to both transformed and untransformed classes?

alfonsogarciacaro commented 3 years ago

A parameter to make it more explicit would be a good idea, but this still won't remove the need of the .module.css extension because this is used by the bundlers to identify that the classes must be mangled (unless you change the bundler configuration): https://webpack.js.org/loaders/css-loader/#modules

Something like style_module.clickable || "clickable" should work with both global and mangled classes, although it would always generate less clean code for the rare case (I think) of using global classes from a css module. I assume the CSS parser cannot identify the :global marker, right? In that case, we could generate a nested class with the same properties but erasing to the unmangled classs strings (as the TP does normally), so users can do:

MyCss.Clickable // mangled class
MyCss.Global.Clickable // erases to "clickable"
zanaptak commented 3 years ago

Ah, there is exportGlobals setting that provides the global classes in the object if desired. No need to address this in the TP.

Is it correct understanding that the specified CSS file path will be used directly in a JS import statement -- that's why relative path is used so both the TP and Webpack resolution work?

zanaptak commented 3 years ago

Hmm, \ separator works in Webpack on Windows, but needs to be escaped in the JS string, I wonder if it's worth trying some kind of path normalization or escaping in the TP, or just leave it alone and user has to use specific format.

alfonsogarciacaro commented 3 years ago

Ah, there is exportGlobals setting that provides the global classes in the object if desired. No need to address this in the TP.

Not entirely sure how exportGloblals work, but it's good that Webpack can deal with that :)

Is it correct understanding that the specified CSS file path will be used directly in a JS import statement -- that's why relative path is used so both the TP and Webpack resolution work?

Yes, that's correct. My idea is that users would type the path as it must appear in JS and we can use that to identify if it should be treated as a CSS module. The TP can do some processing like normalizing the path separator but the most important thing is the path must be relative to the source file and unfortunately I couldn't find a way for the TP to know the directory of the .fs file creating the type (it seems the working directory for the TP becomes the directory of the .fsproj file). That's why users need to pass manually resolutionFolder=__SOURCE_DIRECTORY__.

zanaptak commented 3 years ago

@alfonsogarciacaro I've pushed some changes to the branch and am essentially ready to merge.

It is no longer automatically triggered by filename but I prefer the explicitness of the parameter and avoiding any surprises due to "magic" since this library overall is not specific to webpack or client-side.

The new parameter fableCssModule must be specified to enable the feature.

No longer expecting certain path format or filename extension in the TP, it will be up to user to specify as needed. I'll document the expected configuration for the option, and I feel that the behavior of webpack reporting any error message failing to open module during compile and ability to inspect the JS is sufficient for troubleshooting. This also allows for non-default css-loader configuration in case they are not using .module.css extension for whatever reason.

For backslashes I chose only to escape the source path which is purely mechanical fix to avoid syntax error on Windows. Again it will be user responsibility to specify correctly for desired development platform if necessary, using cross-platform separators or the platform-specific parameter capability I've previously built into the TP.

Please let me know if you have further thoughts or issues.

alfonsogarciacaro commented 3 years ago

Hi @zanaptak! I missed your previous comment, sorry! Those are great changes, thanks for merging!