Use easy-to-test CSS modules

[pjanik]

I think that our main problem about CSS modules was that it's making integration testing really annoying because of the random hash.

This configuration setups constant prefix, which is still unique enough to be sure that we'll avoid 99.99% of conflicts with outer or inner apps/components.

It also makes testing possible, CSS classes are not pretty, but won't change randomly. If we ever need to change this prefix, an update of tests will be trivial too.

Not pushing for it, this PR is opinionated and more like a proposal. But I think it gives us most of the CSS modules advantages and removes the major issue/pain. Does it seem like a reasonable compromise?

[scytacki]

I think it is good idea. But I do want to hear the opinions of others: @kswenson, @ekosmin, @knowuh, @sfentress, @dstrawberrygirl, @DALove1025, @mklewandowski, and @dougmartin

[knowuh]

I am in favor of predictable & meaningful class names, regardless of testing actually.

I was also in favor of the data attribute approach where items that are useful to tests are called out, eg: data-cy="open-button". There are problems with that approach too. Most notably it requires advanced knowledge about what divs are going to be useful to tests. And also its more verbose, and doesn't help the deployed application (so it might also seem a bit wasteful).

In either event, these are not mutually exclusive, and I like removing the random hash from our class names.

This will all become irrelevant once I convert everyone to using styled components anyway. 😜

[dstrawberrygirl]

I think this is a pretty good idea - a small thing to note is that search/replacing starter-projects as suggested in the ReadMe would leave the -v1 in the prefix, which brings up a larger question of versioning in other projects. Would -v1 remain as a reminder that the new project is based on the original v1 starter project, or would it relate to the version of the new project? Perhaps some clarification in the ReadMe would help here.

[pjanik]

This -v1 would be only in CSS class, and it's more like a CSS-suffix-version. Honestly, I think we can drop it. I can't really imagine use case when we need to change this suffix, but then we can just change it from 'your-project' to 'your-project-2' or 'your-project-<random_hash>' etc. Perhaps doesn't make sense to plan it too much before having any use case.

[kswenson]

I agree that as an implementation of CSS modules this seems like a reasonable approach. I don't feel like as an organization we've settled on the fact that CSS modules is the preferred standard, however. My inclination would be to make it opt-in rather than on by default.

[pjanik]

I'm just working without CSS modules for the first time in a long time, in the DESE models repo. And I've managed to have class name conflicts within my small app twice. It's so easy. Even if you nest classes, it's enough that you use the same name somewhere in the ancestor component. And there's no easy way to prevent it (unless doing namespacing manually, but why?). Not saying CSS modules are the best approach out there, but I feel like it's just better than the no-modules approach. 😉

[pjanik]

Is it time to adopt this pattern? 😉
DESE proved that messing with non-scoped CSS classes can have unpleasant long-term consequences. Also, I regularly encounter CSS conflicts, even in very small apps.

[dougmartin]

@pjanik I think it makes sense but we should remove the -v1 suffix on the generated css until we have a use case for it.

[pjanik]

@dougmartin, the reason for __starter-projects-v1__ was to avoid content-based hashing, as many of our QA engineers use these class names with semi-random hash suffixes in QA automation. I have still seen this happening. I believe this hash is related to the selector name and file path, so any refactoring can completely break QA. I probably need to read more about how it's generated. A constant suffix is a workaround for that while providing some project-wide scoping in the unlikely situation when two projects without any hash suffixes could clash. However, if you're referring to the v1 itself, that can obviously be removed. If we ever need to change it to -v2 for any reason, it can be done anyway.