Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(core): add support for CSS modules to dev server + lib build #7650

Merged
merged 1 commit into from
Nov 22, 2019

Conversation

erikmunson
Copy link
Member

Don't be fooled by the line/file count, this PR was the product of a full 1.5 days of discovery and glorious learning.

At the most recent UI SIG meeting, we aligned around the idea of experimenting with CSS Modules in a manner that would limit the blast radius of changes in the event we decided to change course after experimentation. The current thinking is that we'll:

  1. Setup CSS Modules in @spinnaker/core (but not across all packages)
  2. Use CSS Modules to build upcoming reusable presentation components (modals first, then maybe tables and/or navigation, etc.)
  3. Use those presentation components in real-world views and learn about where CSS Modules are good/bad/ugly
  4. Re-evaluate in a few months once we've done 1-3 and decide on the next set of experimentation / work

This PR accomplishes 1 in what I hope is a manner that will work as desired even as we move into other packages and potentially share styles between packages. The two primary components we need are the ability to parse CSS Module files and load them properly, and the ability to have TypeScript know about the CSS Modules being imported.

For loading CSS Modules, the standard webpack css-loader is the preferred method and works great. You just turn it on and it does the thing. So that's what I did.

For TypeScript support, the story is less straightforward. Some folks only go as far as declaring a blanket type of { [className: string]: string } for all CSS module files, which means you don't get autocomplete and TS can't alert you to broken usages of class names when things change (bad). The most popular way of getting real TS support is to emit .d.ts files for each CSS file, but in order for TS to properly handle the declaration files they have to be co-located next to source files. Some folks take the stance of generating the files and checking them into source, then verifying they're correct in a CI step because they want to parallelize TS + webpack, but that seems pretty heavy and like overkill for us. Create React App takes the approach of using a TypeScript plugin which is great, but plugins only work at the local dev flow level and don't have any impact during actual compilation.

The approach I took instead was to generate declaration files as part of the dev server build so things work during dev, and use the corresponding CLI tool to output defs in the library build before TypeScript runs so it has the defs available when type checking. With both of those in place it's possible to put the declaration files in .gitignore, so while they will show up in your local dev environment they won't make diffs noisy or require a confusing generate + verify workflow. A couple other things:

  • Can other packages like amazon still build properly now that core has module shenanigans going on? Yes, because the library build of core does all the bundling, nothing really changes from the perspective of other packages. There's still the usual compiled CSS (which will end up having some auto-generated class names), and the magic strings that come with CSS Module imports are bundled into the lib.js file inline. The CSS Module files themselves are omitted from the built output just like our existing less/css files today.
  • What about importing class names out of re-usable stuff inside core from a package like amazon — will that work in the future if we want it? Yes, because in order to consume something from core it has to be explicitly exported in core. So if I wanted to share some reusable class names outside core, webpack would take the bundled auto-generated class names and make them available in core's exports for other code to consume and the TS compiler would take the types from the .d.ts file and use them to define the type of the export. e.g.:

Modal.tsx (source):

import * as styles from 'Modal.module.css';

export const exportedClassName = styles.header;

Modal.d.ts (built type def):

export declare const exportedClassName: string;

Copy link
Contributor

@alanmquach alanmquach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@erikmunson erikmunson merged commit dbc3eb5 into spinnaker:master Nov 22, 2019
@erikmunson erikmunson deleted the add-css-modules-to-core branch November 22, 2019 00:47
@maggieneterval
Copy link
Contributor

Don't be fooled by the line/file count, this PR was the product of a full 1.5 days of discovery and glorious learning.

I am impressed this didn't take more like 15 days of discovery and glorious learning. 🎉 🎖️ 💯 ❤️ 🤗 📈

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants