Persistent cache #667
Comments
|
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). |
|
Thanks for responding, @jquense! If you’re open to it, I could have a look at implementing a persistent cache through that vendored library? |
|
yeah definitely open to it! |
|
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 sysgears/webpack-virtual-modules#76 but it's not a trivial issue... |
|
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... |
|
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. |
|
maybe it's doing some more aggressive caching? I know the older cache-loader definitely didn't work with astroturf on subsequent runs |
|
Ok so v1 has a |
|
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? |
I've found one issue with it -- this functionality: throws the following error, only when the new loader is enabled: |
|
Aaron is it reproducible with the |
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 |
|
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 💯 |
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. |
|
@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) |
|
@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 |
|
@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
],
} |
|
ok the code looks good, you might need to upgrade css-loader, make sure you have the latest version |
|
@jquense I do. works until altLoader is used. This code may not reproduce in isolation. It may require a large project. |
1 similar comment
|
@jquense I do. works until altLoader is used. This code may not reproduce in isolation. It may require a large project. |
|
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? |
|
Ohhh... I think the class names are colliding. the |
|
@jquense each of those |
|
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 |
|
@jquense I'm on css-loader 5.2.4. |
|
arg very confusing. if it's possible to put a repro together i can inspect that would help a ton |
|
Got lucky... reproduces in this: https://github.com/jsg2021/astroturf-classname-collsion-repro |
|
@jquense if you comment out useAltLoader that repo produces classes I as I would expect... with it on, however, the classes become overlapped. |
|
ok it seems like i had already fixed it and released it as |
|
This classname collision bug just happened to me with 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 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. |
|
order shouldn't matter, it's mostly due to what css-loader hashes. What is your css-loader options and version? |
|
It's Next's built-in CSS handling, which I think uses their own internal fork of css-loader. ( 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 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 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 } } ] }
],
} |
|
next is probably using an old css-loader that doesn't support this :/ |
|
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 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! |
|
A repro would be awesome, that's the easiest way to troubleshoot! |
|
I've provided a repro in #706! |
|
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 #706 (comment). 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. |
|
hey, no i've not had a chance to look over this, may be able to look this week |
|
@lostfictions I took a very quick look of the latest repro. The reason it's broken is because the |
|
@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 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 |
|
I don't own the next-astroturf stuff but would love to see that happen. Anyone want to work on it? |
|
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 |
|
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 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 |
|
you need to add a |
|
yeah I had |
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
cssAPI (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?
The text was updated successfully, but these errors were encountered: