jest-preview icon indicating copy to clipboard operation
jest-preview copied to clipboard

[CSS REWRITE] [BIG UPDATE] Change how to process CSS

Open nvh95 opened this issue 2 years ago • 4 comments

What is it?

Current approach to process CSS

  • Currently, we use Jest Code Transformer to transform CSS. Except Pure CSS, we transform different CSS languages such as Sass, CSS Modules, TailwindCSS, PostCSS... then embed them directly the output CSS to the document.head of jsdom. "Jest Preview server" will serve jsdom as real DOM, so we can see the CSS on the "Jest Preview Dashboard". However, this approach encounters a number of problems
    • (The most annoying one) Jest Code Transformer does not support async actions by default. That means the code must be in "SYNC" style. It's very hard to do some asynchronous actions in a synchronous manner . Sometimes, I could find the sync API to use. Otherwise, I ended up with spawning a new node process, console.log the output the stdout, then reading/ parsing to get the returned value. This is a tricky one and does not scale, and I considered it as a unscale code. https://github.com/nvh95/jest-preview/blob/d991e54382ae6e4c9fb697bdcc8a2b147cef8b23/src/transform.ts#L21-L42
    • After #241, the time processing CSS file takes is long. For example, processing a CSS file in a project with PostCSS configuration would take 0.3 seconds on Macbook Air M1. If the user test case contains 10 CSS files, the total time would be 3 seconds. I don't like the idea of waiting 3 seconds before a user's test can run. To be fair, this is not a critical problem, since Jest will cache the computation result, so it's likely you only need to wait once.

Is there any better solution

Yes. But first, let me describe how I come up with that idea.

A few days ago, I was working on #230. The issue is that when a CSS file that contains an @import statement, jest-preview could not load the content of the imported file:

// index.css. This file is imported to the setupTest.js
@import "base.css";
@import "components.css";
@import "utils.css";

If you are curious, I handled this in #248, by inline the imported CSS, using (postcss-import). But the thing is, when opening Jest Preview Dashboard my index.css will automatically request CSS files that are imported. (It's a CSS standard)

Response:

This opens a new horizon for me to resolve all those issues above. What if instead of processing CSS at Jest Code Transformation, the "Jest Preview Dashboard" will process the CSS? I was in the middle of dinner with my family and I had to stop to think and draft the solution.

An HTTP-based approach

This is briefly how it would work:

  • Jest Preview does not process CSS anymore. It just inject the URL to the jsdom's document.head. Like this:
function processCss(src, filename) {
  return {
    code: `const relativeCssPath = "${relativeFilename}";
          const link = document.createElement('link');
          link.rel = 'stylesheet';
          link.href = relativeCssPath;
          document.head.appendChild(link);
          module.exports = JSON.stringify(relativeCssPath);`,
  };
}

  • "Jest Preview Dashboard" will request relativeCssPath from "Jest Preview Server"
  • "Jest Preview Server" will process the CSS and return it to the browser ("Jest Preview Dashboard") => User can still see the CSS, the same way it is implemented in Code Transformation.

By doing this, we are moving the computation from Jest process to our "Jest Preview Server" process. Let's see the pros and cons Pros

  1. Jest will run faster the first time and in the CI environment (see one of my attempts at #247)
  2. (The most important one) We can write asynchronous code to process CSS
  3. No stale CSS if we update the implementation of the transformer (Jest does not automatically clear cache if code transforms changes. We are clearing the cache in postinstall script, after user installs/ updates jest-preview, see https://github.com/nvh95/jest-preview/issues/51#issuecomment-1221358730)
  4. As a result of 3, we can remove postinstall script, which is considered bad for "security". See https://socket.dev/npm/package/jest-preview

Cons

  1. jsdom does not contain CSS anymore. If someone wants to do some kinds of "visual testing" using jest-preview/transforms/css. It wouldn't be possible. (Or would it be? Can we reuse processCss?)
  2. Redundant CSS processing. Every time "Jest Preview Dashboard" request a CSS file, "Jest Preview Dashboard" needs to compile the CSS. However, we can handle this by some caching mechanism on both the server (in-memory is enough, cache to disk and LRU are OK but would be overkill) and client (browser cache headers like cache-control/ etag) sides.
  3. (Not really a con) The computation does not go anywhere, it just shifts from Jest to "Jest Preview Server".

Are any caveats?

  • Yes. This supposes to be a BREAKING change. But thanks to the architecture of jest-preview, despite the re-architecture, the user experience remains the same. I have only one concern that Jest Code Transform does not invalidate. But I personally think we can handle it by taking the code transformed itself into account in getCacheKey (https://github.com/nvh95/jest-preview/issues/51#issuecomment-1221358730).
  • We might still need to process CSS Modules in Jest Code Transformation because of the cssModulesExportedTokens

Describe alternatives you've considered

Do not implement this at all 😂. Even though the current (Jest Transformation) approach might be slow in CI, but it's not too slow and it's cache-able. If implementing this is too hard. We can just leave it AS-IS

Additional context

#241 #247

nvh95 avatar Aug 23 '22 10:08 nvh95

Hey @ntt261298, please see my note above on a new approach to handle CSS. It would be great if you can give some of your thoughts. Thanks.

nvh95 avatar Aug 23 '22 10:08 nvh95

I think this approach is really promising. (Reduce the test running time, Be able to handle async tasks so we can scale easier in the future,…) I still have some thoughts about this approach:

  • The time for the browser to fully load the CSS of the preview might be longer than current (But I think reducing the test running time is more important).
  • We might still need to transform CSS module in the transformer. As I understand, we still need to do the expensive task postcssrc() for every test. What if we process the CSS module in the Jest Preview Dashboard? Then it might take a much longer time for the browser to load CSS fully. (Not sure if the user has enough patience to wait for that). I think this is a trade-off.
  • Reducing the time of running postcssrc() might be the problem we need to solve for both approaches (current and new), but it’s probably the really hard one.

To summarize, I think it’s feasible to implement the new approach and will bring more pros than cons.

ntt261298 avatar Aug 23 '22 16:08 ntt261298

[Just a note] This can fix #260

nvh95 avatar Sep 08 '22 15:09 nvh95

hi, is this on the roadmap? i would like to reduce the runtime in CI or make it only parse css based on some env variable. looks like this rewrite would solve that.

szamanr avatar Oct 31 '23 05:10 szamanr