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

In-memory cache isn't compatible with thread-loader #881

Closed
malash opened this issue Dec 3, 2021 · 3 comments
Closed

In-memory cache isn't compatible with thread-loader #881

malash opened this issue Dec 3, 2021 · 3 comments
Labels
bug report 🦗 Issue is probably a bug, but it needs to be checked bundler: webpack 📦 Issue is related to webpack bundler cat: performance 🚀 Issue is related to performance needs: complete repro 🖥️ Issue need to have complete repro provided

Comments

@malash
Copy link
Contributor

malash commented Dec 3, 2021

Environment

  • Linaria version: 2.2.0
  • Bundler (+ version): Webpack 5.64.4
  • Node.js version: 16.3.0
  • OS: macOS 12.0.1

Description

The PR #879 introduced an in-memory cache to fix rebuild issue and accelerate bundle time. It's awesome, but when I try to integrate the latest version of Linaria in my project, I found some output CSS code is missing.

I deep dived and realize the in-memory cache may not be compatible with thread-loader. thread-loader use a thread poll to execute loaders in non-main threads, which can hugely speed up Webpack's building time in a large project. Since the threads in NodeJS cannot share the same closure context, the cssLookup map could be empty when running outputCssLoader.ts in a brand new thread.

I'm not sure is this a bug or a feature request? because not all Webpack projects use the thread-loader. But in our project, the build time would become non-acceptable if I disabled the thread-loader. So I can provide a posable solution if you agree to fix this issue:

Solution

Add a new options to return the right of choice to Linaria users.

  • cacheType: 'memory' | 'filesystem' (default: 'memory'):

This option is inspired from Webpack's cache.type. If set to memory the loader works in the same way as 2.2.0, and if set to filesystem it works in the same way as 2.1.0.

  • cacheDirectory: string (default: '.linaria-cache'):

Same as previous. Only works when cacheType: 'filesystem'

Reproducible Demo

https://github.com/malash/linaria-issue-881

You can enable/disable this line and run yarn webpack, the check the result of dist/styles.css.

image

@malash malash added bug report 🦗 Issue is probably a bug, but it needs to be checked needs: complete repro 🖥️ Issue need to have complete repro provided needs: triage 🏷 Issue needs to be checked and prioritized labels Dec 3, 2021
@github-actions github-actions bot added bundler: webpack 📦 Issue is related to webpack bundler cat: performance 🚀 Issue is related to performance and removed needs: triage 🏷 Issue needs to be checked and prioritized labels Dec 3, 2021
@malash
Copy link
Contributor Author

malash commented Dec 7, 2021

cc @Anber

@Anber
Copy link
Collaborator

Anber commented Dec 9, 2021

Hmm…
thread-loader docs say, that loaders in threads have some limitations:

  • Loaders cannot emit files.
  • Loaders cannot use custom loader API (i. e. by plugins).
  • Loaders cannot access the webpack options.

I'll try to implement a configurable cache, but it looks like we can face more problems with this plugin.

@malash
Copy link
Contributor Author

malash commented Dec 13, 2021

Hi @Anber I cost some time to implement this feature in #884 for Linaria v2. Could you please take a look ?

If it's approved, I can create another PR to cherry pick it into Linaria v3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug report 🦗 Issue is probably a bug, but it needs to be checked bundler: webpack 📦 Issue is related to webpack bundler cat: performance 🚀 Issue is related to performance needs: complete repro 🖥️ Issue need to have complete repro provided
Projects
None yet
Development

No branches or pull requests

2 participants