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

Add CSS Modules in Build SSR #3170

Merged
merged 1 commit into from
Apr 16, 2021
Merged

Add CSS Modules in Build SSR #3170

merged 1 commit into from
Apr 16, 2021

Conversation

drwpow
Copy link
Collaborator

@drwpow drwpow commented Apr 16, 2021

Changes

CSS Modules, when generated by Snowpack, are missing from SSR builds. This PR is a quick unblocker to get that working. This PR only affects the build & SSR responses, and leaves dev server as-is.

Problem

The problem with Snowpack and CSS Modules is two-fold:

  1. Snowpack doesn‘t have full support for nested extensions (.module.css). We don’t have the ability to target behavior based on a nested extension. This is good when you do want to run the same CSS passes on everything. But bad for CSS Modules when we need an extra step to run there and only there.
  2. Snowpack generates the CSS Module response within the .module.css.proxy.js file. This means during SSR, we end up with an incorrect .css file that doesn’t match the transformed values. We can’t just run the proxy file, either, as that tries to add <style> tags to a DOM that doesn’t exist (yet).

This PR

This PR doesn’t address #1, as that’s a much larger, more sweeping change across Snowpack that also has plugin implications. But it does resolve #2, moving the CSS Module pass into a centralized place that transforms the CSS. And then it updates the proxy file to use the new CSS Module code.

This PR doesn’t change the dev server response. It does change the build result, but not in a way that affects users (the output CSS now has the correct CSS Module names, which is expected, and there’s an addition of a .module.css.json file with module names, but it is only used if users manually do something with it). This PR does fix the SSR response so that the correct transformed CSS is not only output; it’s included in the css array for response.

Testing

✅ Tested dev server and it works as it did before
Screen Shot 2021-04-15 at 22 25 36
✅ Updated our css-modules build test to ensure build is working, too.

Docs

No docs needed; behind-the-scenes

@drwpow drwpow requested a review from a team as a code owner April 16, 2021 16:26
@vercel
Copy link

vercel bot commented Apr 16, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/pikapkg/snowpack/6JXdeEKmdRwtLZQpkGuPV84h9LfZ
✅ Preview: https://snowpack-git-css-modules-ssr-pikapkg.vercel.app

result
.warnings()
.forEach((element) => logger.warn(`${url} - ${element.text}`, {name: 'snowpack:cssmodules'}));
// return the JS+CSS proxy file.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

First step in this PR is to centralize the CSS Module action, and make it happen 1 step earlier in the CSS. Now we persist the CSS Module results in build & SSR, rather than living only in the proxy file.

},
};
// handle CSS Modules, after plugins run
if (srcPath.endsWith('.module.css')) {
Copy link
Collaborator Author

@drwpow drwpow Apr 16, 2021

Choose a reason for hiding this comment

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

Second step is to add a step in the build pipeline only for .module.css files. While this isn’t ideal long-term, it’s non-disruptive to existing plugins and Snowpack users, and it’s currently the only method we have to target only these files. It can be easily-removed when we add full nested extension support, but even then we may still retain some of this special handling because of Snowpack’s internal understanding of proxy files.

import postCssModules from 'postcss-modules';
import {logger} from '../logger';

const cssModuleNames = new Map<string, string>();
Copy link
Collaborator Author

@drwpow drwpow Apr 16, 2021

Choose a reason for hiding this comment

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

The third step is to keep existing behavior. How do we store the JSON name mapping that CSS Modules generates? Before it was kept in the proxy response, but now that it’s centralized it has to be temporarily stored somewhere else.

We do now build a .module.css.json file with this info, which we could import in the proxy file. While that works, and may be technically correct, it would be a change in current behavior as it adds an additional request for every component (which may not get bundled away at the end, thus impacting users).

So instead, we simply keep the JSON output in memory until it‘s needed. This memory gets blown away after every build. In the dev server, only the CSS Modules loaded live in memory, and doesn’t leak with HMR (same source files just get updated responses). It shouldn’t cause any issues for Snowpack users, and it keeps existing behavior as-is, which is important.

// CSS Modules: include both CSS and JS
const cssSource = source.replace(/\.proxy\.js$/, '');
css.push(cssSource);
deps.push({name, source});
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This last change is the important part for SSR. Now that we generate the correct CSS, we need to add it to the SSR output.

Copy link
Contributor

@matthewp matthewp left a comment

Choose a reason for hiding this comment

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

lgtm

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

Successfully merging this pull request may close these issues.

It never finishes installing
2 participants