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

fix(webpack): replace file system cache with in-memory cache (fixes #878) #879

Merged
merged 3 commits into from Dec 1, 2021

Conversation

Anber
Copy link
Collaborator

@Anber Anber commented Nov 30, 2021

Motivation

Current implementations of webpack loaders use file system cache for storing evaluated css-files. It causes a few problems with URLs in CSS, with HMR and even with finding a root of a project (if Linaria is used in monorepo). At the same time, Rollup loader uses in-memory cache that works pretty good.

Summary

In #878 @malash suggested using inline matchResource in order to fix doubled reloads. It appeared, that with such a tool we can finally get rid of filesystem cache and migrate to in-memory cache, which seems to be much more problemless.

@callstack-bot
Copy link

callstack-bot commented Nov 30, 2021

Hey @Anber, thank you for your pull request 🤗.
The coverage report for this branch can be viewed here.

@Anber
Copy link
Collaborator Author

Anber commented Nov 30, 2021

Hi @malash,
I need your opinion here :) How do you think, will it work?

Copy link
Contributor

@malash malash left a comment

Choose a reason for hiding this comment

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

Thanks @Anber for your great work! This PR is awesome and in-memory cache must provide better performance than file system cache.

I'v review the PR and leave some comments.

B.T.W. Do you have plan to cherry-pick this PR into Linaria v2, since it's still the stable branch in NPM ?

packages/webpack4-loader/src/index.ts Outdated Show resolved Hide resolved
packages/webpack4-loader/src/outputCssLoader.ts Outdated Show resolved Hide resolved
@Anber
Copy link
Collaborator Author

Anber commented Dec 1, 2021

Do you have plan to cherry-pick this PR into Linaria v2

I didn't have, before you asked for :) I'll release it in v2 as well.

@ntucker
Copy link

ntucker commented Jan 4, 2022

What test can I use to detect linaria css files now that the .linaria-cache directory is not a test?

PS) Would be nice to publicize breaking changes like this and migration paths.

@ntucker
Copy link

ntucker commented Jan 4, 2022

Figured it out for others looking: /\.linaria\.css$/i

@ntucker
Copy link

ntucker commented Jan 6, 2022

A better solution for double rebuilds is to add a watch ignore as tricking webpack into think css is a js file breaks a lot of assumptions other webpack tools will make - including the popular react-refresh

@malash
Copy link
Contributor

malash commented Jan 10, 2022

@ntucker I did try watch ignore in previous version and it caused CSS reloading fail to work.

I prefer inline matchResource because it is an officially recommended way to support CSS-in-JS in Webpack. However 3rd-party libraries have to take some efforts to support virtual files.

@ntucker
Copy link

ntucker commented Jan 10, 2022

@malash can you point me to the docs where they mention this usage with css-in-js? All I see is the warning:

"It is not recommended to use this syntax in application code. Inline request syntax is intended to only be used by loader generated code. Not following this recommendation will make your code webpack-specific and non-standard."

And even after diving it - it's unclear how to detect these properly so they aren't treated as JS files.

Specifically the very popular react-refresh plugin doesn't seem to work with it #897

@malash
Copy link
Contributor

malash commented Jan 10, 2022

@ntucker This demo code is an example of CSS-in-JS. It use comment to contain the CSS source.

image

As i understand in that sentence application code means client side code that run on web browsers, while loader generated code returned from the loader function is ok to use inline matchResource syntax. It aims to make sure any UI component can be shared between Webpack and another bundling tools, like Rollup.js. But it's not a limitation for this PR.

And even after diving it - it's unclear how to detect these properly so they aren't treated as JS files. Mostly agree. I can find few documentation of virtual file, and no one explain it. Maybe you can create an issue to Webpack ? cc @sokra

@ntucker
Copy link

ntucker commented Jan 12, 2022

This is the full moduleData of a css module created in this way (from console log in https://github.com/pmmmwh/react-refresh-webpack-plugin/blob/ebe885a1df2cd528645b7cc68dfc4fa59dde4d11/lib/utils/injectRefreshLoader.js) - while it's getting some of the correct loaders, it shows up as type: 'javascript/auto', which makes it impossible to distinguish from other js files

{
  layer: null,
  request: '/home/ntucker/src/anansi/node_modules/mini-css-extract-plugin/dist/loader.js??ruleSet[1].rules[6].oneOf[1].use[0]!/home/ntucker/src/anansi/node_modules/css-loader/dist/cjs.js??ruleSet[1].rules[6].oneOf[1].use[1]!/home/ntucker/src/anansi/node_modules/postcss-loader/dist/cjs.js??ruleSet[1].rules[6].oneOf[1].use[2]!/home/ntucker/src/anansi/node_modules/@linaria/webpack5-loader/lib/outputCssLoader.js?cacheProvider=%2Fhome%2Fntucker%2Fsrc%2Fanansi%2Fpackages%2Fwebpack-config-anansi%2Flib%2Fbase%2FlinariaFileCache.js!/home/ntucker/src/anansi/examples/linaria/src/MenuItem.tsx',
  userRequest: '/home/ntucker/src/anansi/examples/linaria/src/MenuItem.linaria.css!=!/home/ntucker/src/anansi/node_modules/@linaria/webpack5-loader/lib/outputCssLoader.js?cacheProvider=%2Fhome%2Fntucker%2Fsrc%2Fanansi%2Fpackages%2Fwebpack-config-anansi%2Flib%2Fbase%2FlinariaFileCache.js!/home/ntucker/src/anansi/examples/linaria/src/MenuItem.tsx',
  rawRequest: './MenuItem.linaria.css!=!../../../node_modules/@linaria/webpack5-loader/lib/outputCssLoader.js?cacheProvider=%2Fhome%2Fntucker%2Fsrc%2Fanansi%2Fpackages%2Fwebpack-config-anansi%2Flib%2Fbase%2FlinariaFileCache.js!./MenuItem.tsx',
  loaders: [
    {
      loader: '/home/ntucker/src/anansi/node_modules/mini-css-extract-plugin/dist/loader.js',
      options: [Object],
      ident: 'ruleSet[1].rules[6].oneOf[1].use[0]'
    },
    {
      loader: '/home/ntucker/src/anansi/node_modules/css-loader/dist/cjs.js',
      options: [Object],
      ident: 'ruleSet[1].rules[6].oneOf[1].use[1]'
    },
    {
      loader: '/home/ntucker/src/anansi/node_modules/postcss-loader/dist/cjs.js',
      options: [Object],
      ident: 'ruleSet[1].rules[6].oneOf[1].use[2]'
    },
    {
      loader: '/home/ntucker/src/anansi/node_modules/@linaria/webpack5-loader/lib/outputCssLoader.js',
      options: 'cacheProvider=%2Fhome%2Fntucker%2Fsrc%2Fanansi%2Fpackages%2Fwebpack-config-anansi%2Flib%2Fbase%2FlinariaFileCache.js',
      ident: undefined
    }
  ],
  resource: '/home/ntucker/src/anansi/examples/linaria/src/MenuItem.tsx',
  context: '/home/ntucker/src/anansi/examples/linaria/src',
  matchResource: '/home/ntucker/src/anansi/examples/linaria/src/MenuItem.linaria.css',
  resourceResolveData: {
    _ResolverCachePluginCacheMiss: true,
    context: {
      issuer: '/home/ntucker/src/anansi/examples/linaria/src/MenuItem.tsx',
      issuerLayer: null,
      compiler: undefined
    },
    path: '/home/ntucker/src/anansi/examples/linaria/src/MenuItem.tsx',
    request: undefined,
    query: '',
    fragment: '',
    module: false,
    directory: false,
    file: false,
    internal: false,
    fullySpecified: false,
    descriptionFilePath: '/home/ntucker/src/anansi/examples/linaria/package.json',
    descriptionFileData: {
      name: 'example-linaria',
      version: '3.0.21',
      private: true,
      scripts: [Object],
      devDependencies: [Object],
      dependencies: [Object],
      browserslist: [Array]
    },
    descriptionFileRoot: '/home/ntucker/src/anansi/examples/linaria',
    relativePath: './src/MenuItem.tsx',
    __innerRequest_request: undefined,
    __innerRequest_relativePath: './src/MenuItem.tsx',
    __innerRequest: './src/MenuItem.tsx'
  },
  settings: { type: 'javascript/auto' },
  type: 'javascript/auto',
  parser: JavascriptParser {
    hooks: {
      evaluateTypeof: [HookMap],
      evaluate: [HookMap],
      evaluateIdentifier: [HookMap],
      evaluateDefinedIdentifier: [HookMap],
      evaluateCallExpressionMember: [HookMap],
      isPure: [HookMap],
      preStatement: [Hook],
      blockPreStatement: [Hook],
      statement: [Hook],
      statementIf: [Hook],
      classExtendsExpression: [Hook],
      classBodyElement: [Hook],
      classBodyValue: [Hook],
      label: [HookMap],
      import: [Hook],
      importSpecifier: [Hook],
      export: [Hook],
      exportImport: [Hook],
      exportDeclaration: [Hook],
      exportExpression: [Hook],
      exportSpecifier: [Hook],
      exportImportSpecifier: [Hook],
      preDeclarator: [Hook],
      declarator: [Hook],
      varDeclaration: [HookMap],
      varDeclarationLet: [HookMap],
      varDeclarationConst: [HookMap],
      varDeclarationVar: [HookMap],
      pattern: [HookMap],
      canRename: [HookMap],
      rename: [HookMap],
      assign: [HookMap],
      assignMemberChain: [HookMap],
      typeof: [HookMap],
      importCall: [Hook],
      topLevelAwait: [Hook],
      call: [HookMap],
      callMemberChain: [HookMap],
      memberChainOfCallMemberChain: [HookMap],
      callMemberChainOfCallMemberChain: [HookMap],
      optionalChaining: [Hook],
      new: [HookMap],
      expression: [HookMap],
      expressionMemberChain: [HookMap],
      unhandledExpressionMemberChain: [HookMap],
      expressionConditionalOperator: [Hook],
      expressionLogicalOperator: [Hook],
      program: [Hook],
      finish: [Hook]
    },
    sourceType: 'auto',
    scope: undefined,
    state: undefined,
    comments: undefined,
    semicolons: undefined,
    statementPath: undefined,
    prevStatement: undefined,
    currentTagData: undefined
  },
  parserOptions: undefined,
  generator: JavascriptGenerator {},
  generatorOptions: undefined,
  resolveOptions: undefined
}

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.

None yet

4 participants