Persistent cache

[makenosound]

Hello! I’ve started playing around with astroturf for a slightly strange project. It’s a library that exports a lot of individual atomic classes using the css API (about 10,000) and so the start-up penalty for astroturf is pretty high. Perfromance is remarkably good once that initial load is complete through.

I’m wondering if it’d be possible to leverage a disk cache alongside the in-memory cache for webpack so that some of that work can be avoid for subsequent startups?

[jquense]

I am hoping that webpack5's persistent cache we give us this but we need to figure out how to make it work with the in memory one. We are using a slightly forked and vendored version of https://github.com/sysgears/webpack-virtual-modules tho which doesn't yet support this (and i've not had the bandwidth to figure it out myself).

[makenosound]

Thanks for responding, @jquense! If you’re open to it, I could have a look at implementing a persistent cache through that vendored library?

[jquense]

yeah definitely open to it!

[lostfictions]

Would love to have something figured out for this -- I switched to webpack 5 to try to improve long startup times on our project by leveraging the persistent cache, but it seems like it's erroring out on subsequent runs trying to locate the cached versions of the virtual css modules.

Looks like they're trying to figure this out over at https://github.com/sysgears/webpack-virtual-modules/issues/76 but it's not a trivial issue...

[jquense]

Hmm I've not run into actual errors with webpack 5s persistent cache. I know astroturf files aren't in it, but I'd assumed it was just missing the cache and falling back to reprocessing everything. Tho perhaps I've just set it up wrong...

[lostfictions]

It's just as likely it's something in my setup -- I'm using electron-forge which brings its own webpack defaults, so there might be other stuff going on. I could check and see if it repros with a more basic setup.

[jquense]

maybe it's doing some more aggressive caching? I know the older cache-loader definitely didn't work with astroturf on subsequent runs

[lostfictions]

@jquense Turns out it repros for me with a very minimal setup! I've opened a new issue since the fact that it errors out seems distinct from this question of supporting persistent caching: #681

[jquense]

Ok so v1 has a useAltLoader option you can pass in your webpack config for astroturf/loader. This fundamentally changes the approach astroturf takes with webpack, and as a consequence adds persistent cache support. We've been trying it our for a few weeks and it seems to work great, but i'd love more input if you want to give it a whirl

[lostfictions]

Amazing! I'll try to give in a shot next time I spin up a greenfield project or have some time to experiment. Seems like good timing, since Next is starting to roll out production-ready Webpack 5 support.

Seems like Astroturf v1 is getting pretty close to being ready?

[AaronBuxbaum]

Ok so v1 has a useAltLoader option you can pass in your webpack config for astroturf/loader. This fundamentally changes the approach astroturf takes with webpack, and as a consequence adds persistent cache support. We've been trying it our for a few weeks and it seems to work great, but i'd love more input if you want to give it a whirl

I've found one issue with it -- this functionality:

import { stylesheet } from 'astroturf';
import merge from 'lodash/merge';

const { fontFamily, textColor } = stylesheet`
  :export {
    fontFamily: $font-family-sans-serif;
    textColor: $text-color;
  }
`;

throws the following error, only when the new loader is enabled:

ERROR in ./src/shared/components/plots/Config.tsx? (./src/shared/components/plots/Config-undefined.module.scss!=!./node_modules/astroturf/inline-loader.js?source=/Users/aaron/Work/web/src/shared/components/plots/Config.tsx&styleId=!./src/shared/components/plots/Config.tsx?)
Module build failed (from ./node_modules/mini-css-extract-plugin/dist/loader.js):
ModuleBuildError: Module build failed (from ./node_modules/sass-loader/dist/cjs.js):
SassError: expected "{".
  ╷
2 │ import merge from 'lodash/merge';
  │                                 ^
  ╵
  src/shared/components/plots/Config.tsx 2:33  root stylesheet
    at processResult (/Users/aaron/Work/web/node_modules/webpack/lib/NormalModule.js:675:19)
    at /Users/aaron/Work/web/node_modules/webpack/lib/NormalModule.js:777:5
    at /Users/aaron/Work/web/node_modules/loader-runner/lib/LoaderRunner.js:399:11
    at /Users/aaron/Work/web/node_modules/loader-runner/lib/LoaderRunner.js:251:18
    at context.callback (/Users/aaron/Work/web/node_modules/loader-runner/lib/LoaderRunner.js:124:13)
    at /Users/aaron/Work/web/node_modules/sass-loader/dist/index.js:62:7
    at Function.call$2 (/Users/aaron/Work/web/node_modules/sass/sass.dart.js:91729:16)
    at _render_closure.call$0 (/Users/aaron/Work/web/node_modules/sass/sass.dart.js:80354:23)
    at Object.Primitives_applyFunction (/Users/aaron/Work/web/node_modules/sass/sass.dart.js:1086:30)
    at Object.Function_apply (/Users/aaron/Work/web/node_modules/sass/sass.dart.js:6007:16)
 @ ./src/shared/components/plots/Config.tsx 2:0-212 6:4-5
 @ ./src/shared/components/plots/Plotly.tsx 11:0-70 44:40-53 44:56-62 47:14-25 47:35-41
 @ ./src/routes/Dashboard/components/DashboardGraph.tsx
 @ ./src/routes/Dashboard/components/DashboardAnalysisGraph.tsx 5:0-46 10:42-56
 @ ./src/routes/Dashboard/components/DashboardPage.tsx 11:0-62 49:38-60
 @ ./src/routes/Dashboard/index.tsx
 @ ./src/routes.tsx 31:0-48 332:6-20
 @ ./src/configureStore.ts 13:0-30 40:38-44
 @ ./src/createApplication.tsx 10:0-46 17:16-30
 @ ./src/clientApplication.tsx 9:0-52 12:9-26
 @ ./src/client.ts

[jquense]

Aaron is it reproducible with the :export {} syntax?

[AaronBuxbaum]

Aaron is it reproducible with the :export {} syntax?

I always get the same error, if that's what you're asking. It works if I use the old loader, the only change is that switchover

[jquense]

ah I think this is because the derived identifier for this is "undefined", can you change it to not destructure and see if it fixes it

[AaronBuxbaum]

ah I think this is because the derived identifier for this is "undefined", can you change it to not destructure and see if it fixes it

This does indeed fix it!

I see one other issue where the new loader creates 1 more CSS file than expected, which causes issues because it changes the way in which some of those affected classes cascade. This may be a combination of the change with some other webpack configuration, or just the loader itself, I'm not sure. Besides that, the system is 💯

[jsg2021]

Ok so v1 has a useAltLoader option you can pass in your webpack config for astroturf/loader. This fundamentally changes the approach astroturf takes with webpack, and as a consequence adds persistent cache support. We've been trying it our for a few weeks and it seems to work great, but i'd love more input if you want to give it a whirl

It was working, but now I'm seeing missing styles... I'll report back with more concrete info after I get my team up and running.

[jsg2021]

@jquense The styles weren't missing, they were being overridden. I'm seeing classes that should be on only one component bleed over to other components defined in the same file. I tried to create a repro in the example dir, but it doesn't seem to work as with the inline loader at all. (class names aren't present in the dom)

[jquense]

@jsg2021 can you post how you've defined styles in that file. I can think of what could produce that but not the situation that would give rise to it. The other helpful detail would be your webpack loader config

[jsg2021]

@jquense This is one example... the file is just a set of parts... the Dot component has the class of Box as well as its own.

export const Container = styled.div`
    cursor: pointer;
    width: 70px;
    height: 70px;
    display: flex;
    align-items: center;
    justify-content: center;

    img,
    svg {
        width: 100%;
    }

    &:global(.flyout-open) {
        background-color: white;
        box-shadow: -1px 0 0 0 #dcdcdc;
        transition: background-color 0.3s, box-shadow 0.3s;
    }
`;

export const Box = styled.div`
    position: relative;
    width: 42px;
    height: 42px;
`;

export const Dot = styled(User.Presence).attrs({ me: true })`
    position: absolute;
    right: 2px;
    bottom: 2px;
`;

the js loader:

{
            test: jsTestExp,
            use: [
                {
                    loader: 'babel-loader',
                    options: {
                    ...
                    },
                },
                {
                    loader: 'astroturf/loader',
                    options: {
                        allowGlobal: true,
                        useAltLoader: true,
                    },
                },
            ]
        },

css loader:

{
    test: /\.css$/,
    sideEffects: true,
    use: [
        'style-loader',
        {
        loader: 'css-loader',
        options: {
            sourceMap: true,
            importLoaders: 1,
            modules: {
                exportGlobals: true,
                exportLocalsConvention: 'camelCase',
                localIdentName: '[local]--[hash:base64:8]',
            },
        }},
        //postCss
    ],
}

[jquense]

ok the code looks good, you might need to upgrade css-loader, make sure you have the latest version

[jsg2021]

@jquense I do. works until altLoader is used. This code may not reproduce in isolation. It may require a large project.

[jsg2021]

@jquense I do. works until altLoader is used. This code may not reproduce in isolation. It may require a large project.

[jquense]

hmm we have this one in a a few fairly large projects and aren't seeing it, so maybe it's not specifically size related, but something with added complexity. if you pull just this file out a with the same webpack config do you get the same issue?

[jsg2021]

Ohhh... I think the class names are colliding.

the .cls2--3LXraOKI in this pick is showing styles from three unique components

[jsg2021]

@jquense each of those .cls2--3LXraOKI are from unique components from the same file.

[jquense]

are you sure you have the latest css-loader? I would expect this to happen on an slightly older release, basically the hash seems like it's not taking into account part of the resource name, which is producing conflicts

[jsg2021]

@jquense I'm on css-loader 5.2.4.

[jquense]

arg very confusing. if it's possible to put a repro together i can inspect that would help a ton

[jsg2021]

Got lucky... reproduces in this: https://github.com/jsg2021/astroturf-classname-collsion-repro

[jsg2021]

@jquense if you comment out useAltLoader that repo produces classes I as I would expect... with it on, however, the classes become overlapped.

[jquense]

ok it seems like i had already fixed it and released it as 1.0.0-beta.21 but for some reason i cannot find that tag. in any case upgrading seemed to fix it

[lostfictions]

This classname collision bug just happened to me with useAltLoader in 1.0.0-beta.21, unfortunately...

I'd written something like this:

const touchNone = css`
  touch-action: none;
`;

then was experimenting, and added another declaration above it:

const touchPan = css`
  touch-action: pan-y;
`;

const touchNone = css`
  touch-action: none;
`;

I was using touchNone in an element and swapped it for touchPan, but it wasn't working -- inspecting the element showed that both classes were being applied, and touchNone was winning out.

Is there a chance that it's getting confused by declaration order? I think the only thing I did differently than normal here is declaring something above an existing declaration.

[jquense]

order shouldn't matter, it's mostly due to what css-loader hashes. What is your css-loader options and version?

[lostfictions]

It's Next's built-in CSS handling, which I think uses their own internal fork of css-loader. (yarn why css-loader only shows the version bundled with astroturf.) I append astroturf to Next's webpack rules in next.config.js like this:

const withPlugins = require("next-compose-plugins");

module.exports = withPlugins(
  [
    () => ({
      webpack(cfg) {
        cfg.module.rules.push({
          test: /\.tsx$/,
          use: [{ loader: "astroturf/loader", options: { useAltLoader: true } }],
        });

        // unrelated loader config elided...

        return cfg;
      },
    }),
    // unrelated next plugins elided...
  ],
  {
    future: {
      webpack5: true,
    },
  }
);

...which has always worked for me. This is with next@10.2.2.

I'll see if I can make a simpler repro at some point. I'm not sure, but I think stopping and restarting Next's dev server actually fixed the issue. So it might be transient, as a result of editing, rather than a persistent failure under a specific config?

If it helps, here are the relevant bits from the two configs (server and client) that Next spits out when I append console.dir(cfg.module, { depth: null }) just before return cfg; in the above snippet.

Server:

{
  rules: [
    // ...
    {
      test: /\.(tsx|ts|js|mjs|jsx)$/,
      include: [
        '/<project root>',
        /next[\\/]dist[\\/]next-server[\\/]lib/,
        /next[\\/]dist[\\/]client/,
        /next[\\/]dist[\\/]pages/,
        /[\\/](strip-ansi|ansi-regex)[\\/]/
      ],
      exclude: [Function: exclude],
      use: {
        loader: 'next-babel-loader',
        options: {
          configFile: undefined,
          isServer: true,
          distDir: '/<project root>/.next',
          pagesDir: '/<project root>/src/pages',
          cwd: '/<project root>',
          cache: false,
          development: true,
          hasReactRefresh: false,
          hasJsxRuntime: true
        }
      }
    },
    // ...
    {
      oneOf: [
        // ...
        {
          sideEffects: false,
          test: /\.module\.css$/,
          issuer: {
            and: [ '/<project root>' ],
            not: [ /node_modules/ ]
          },
          use: [
            {
              loader: '/<project root>/node_modules/next/dist/compiled/css-loader/cjs.js',
              options: {
                importLoaders: 1,
                sourceMap: true,
                esModule: false,
                url: [Function: cssFileResolve],
                import: [Function: import],
                modules: {
                  exportLocalsConvention: 'asIs',
                  exportOnlyLocals: true,
                  mode: 'pure',
                  getLocalIdent: [Function: getCssModuleLocalIdent]
                }
              }
            },
            {
              loader: '/<project root>/node_modules/next/dist/compiled/postcss-loader/cjs.js',
              options: {
                postcssOptions: {
                  plugins: [
                    [Function (anonymous)] { postcss: true },
                    [Function (anonymous)] {
                      postcssPlugin: 'postcss-preset-env',
                      postcssVersion: '7.0.32'
                    }
                  ],
                  config: false
                },
                sourceMap: true
              }
            }
          ]
        },
        // ...
      ]
    },
    // ...
    { test: /\.tsx$/, use: [ { loader: 'astroturf/loader', options: { useAltLoader: true } } ] }
  ]
}

Client:

{
  rules: [
    // ...
    {
      test: /\.(tsx|ts|js|mjs|jsx)$/,
      include: [
        '/<project root>',
        /next[\\/]dist[\\/]next-server[\\/]lib/,
        /next[\\/]dist[\\/]client/,
        /next[\\/]dist[\\/]pages/,
        /[\\/](strip-ansi|ansi-regex)[\\/]/
      ],
      exclude: [Function: exclude],
      use: [
        '/<project root>/node_modules/@next/react-refresh-utils/loader.js',
        {
          loader: 'next-babel-loader',
          options: {
            configFile: undefined,
            isServer: false,
            distDir: '/<project root>/.next',
            pagesDir: '/<project root>/src/pages',
            cwd: '/<project root>',
            cache: false,
            development: true,
            hasReactRefresh: true,
            hasJsxRuntime: true
          }
        }
      ]
    },
    {
      oneOf: [
        // ...
        {
          sideEffects: false,
          test: /\.module\.css$/,
          issuer: {
            and: [ '/<project root>' ],
            not: [ /node_modules/ ]
          },
          use: [
            {
              loader: 'next-style-loader',
              options: { insert: [Function: insert] }
            },
            {
              loader: '/<project root>/node_modules/next/dist/compiled/css-loader/cjs.js',
              options: {
                importLoaders: 1,
                sourceMap: true,
                esModule: false,
                url: [Function: cssFileResolve],
                import: [Function: import],
                modules: {
                  exportLocalsConvention: 'asIs',
                  exportOnlyLocals: false,
                  mode: 'pure',
                  getLocalIdent: [Function: getCssModuleLocalIdent]
                }
              }
            },
            {
              loader: '/<project root>/node_modules/next/dist/compiled/postcss-loader/cjs.js',
              options: {
                postcssOptions: {
                  plugins: [
                    [Function (anonymous)] { postcss: true },
                    [Function (anonymous)] {
                      postcssPlugin: 'postcss-preset-env',
                      postcssVersion: '7.0.32'
                    }
                  ],
                  config: false
                },
                sourceMap: true
              }
            }
          ]
        },
        // ...
      ]
    },
    // ...
    { test: /\.tsx$/, use: [ { loader: 'astroturf/loader', options: { useAltLoader: true } } ] }
  ],
}

[jquense]

next is probably using an old css-loader that doesn't support this :/

[lostfictions]

Maybe! I'm not sure. One of the big-ticket items of Next 10.1/10.2 was Webpack 5 support, specifically highlighting persistent caching. And useAltLoader didn't error out -- like I said, it seemed to work fine for quite a while until the collision bug (which seemed identical to the one already raised in this thread) occurred.

But I think it should be possible to override Next's built-in CSS handling and use stock css-loader instead, so maybe I can confirm that. I'll try to see if I can come up with a repro at some point!

[jquense]

A repro would be awesome, that's the easiest way to troubleshoot!

[lostfictions]

I've provided a repro in #706!

[lostfictions]

Hey @jquense, just thought I'd give another small poke on this -- have you had a chance to look at the repro? The issue isn't Next-specific -- I've provided a minimal repro here demonstrating the issue, as mentioned in https://github.com/4Catalyzer/astroturf/issues/706#issuecomment-852431370. Sounds like I'm not the only one still having this problem, as #705 suggests... Unfortunately it's kind of a dealbreaker on one project at this point, so if it's not resolvable we're looking at migrating to something else.

[jquense]

hey, no i've not had a chance to look over this, may be able to look this week

[jquense]

@lostfictions I took a very quick look of the latest repro. The reason it's broken is because the localIdentName does not contain a hash. Change it to "[path][name]__[local]_[hash:5]" and the classes do not conflict

[lostfictions]

@jquense This makes a lot of sense, thanks! And I think it explains why things are broken in Next with the alternate loader, too -- seems like they use a custom getLocalIdent with a hash that doesn't take the context into account.

Looking around a bit more, it seems like some folks worked out a solution for using Linaria and Next together while leaning on Next's built-in CSS support. It configures Linaria to export under a .linaria.module.css extension and then rewrites Next's existing rules to check for that extension and use it to decide whether to defer to Next's getLocalIdent or not. Kind of a dirty hack, but I guess something similar is probably necessary to get Next working with Astroturf again.

[jquense]

I don't own the next-astroturf stuff but would love to see that happen. Anyone want to work on it?

[lostfictions]

Okay, based on the above I threw together a fix that seems to solve my issues -- it defers to Next's local identifier generation but appends the resource query (ie. the identifier the css tag is assigned to, which is the missing piece of context for correct classname generation) if it finds Astroturf's alt loader in the Webpack request.

I haven't tested this in other scenarios and don't know enough about Webpack to tell if my technique plays well with anything else or is brittle -- so I don't feel too confident offering this as a complete fix without some feedback and further testing. But it could potentially be the start of a drop-in Next plugin to enable Astroturf?

(In the interim, unfortunately it looks like Next's official with-astroturf example was removed a month back due to lack of maintenance -- probably not helped by the perceived Webpack 5 incompatibility, since they rolled the latter out to all users by default soon after.)

[dreyks]

I'm getting has collisions when using alt loader

this happens when there are several styled components in a single js file. alt loader attaches the component name as a query path/to/file.js?ComponentName, but css-loader only considers the resourcePath part (which does not contain the query). Since the cls1/cls2 class names are same for all the components this leads to same hashes https://github.com/webpack-contrib/css-loader/blob/b29d3899f8130e77e1ad6cf4c4c2fe18116abcd1/src/utils.js#L323

I'm not really sure how to proceed with this since css-loader doesn't pass the full resource to webpack and thus webpack's native [query] placeholder doesn't work either

[jquense]

you need to add a [hash] to your local ident convention config in css-loader, it considers the match resource which is set

[dreyks]

yeah I had [hash] but I understand now that I had other issues - match resource being absent due to https://github.com/4Catalyzer/astroturf/issues/727