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

Cache breaks asset module live reloading #13275

Closed
stefan-toubia opened this issue Apr 30, 2021 · 23 comments
Closed

Cache breaks asset module live reloading #13275

stefan-toubia opened this issue Apr 30, 2021 · 23 comments

Comments

@stefan-toubia
Copy link

Bug report

What is the current behavior?
Live reloading breaks for assets using asset/resource rule type when caching is enabled.

If the current behavior is a bug, please provide the steps to reproduce.

Add an asset loader rule:

      {
        test: /\.(png|svg|jpg|jpeg|gif)$/i,
        type: "asset/resource",
      },

Import an asset

import React from "react";
import ReactDOM from "react-dom";
import logoSrc from "./assets/logo.png";

import "./styles.less";

const App = (): JSX.Element => (
  <div>
    <img src={logoSrc} alt="logo" />
    <div>Hello, React!</div>
  </div>
);

ReactDOM.render(<App />, document.getElementById("root"));

The initial page load works as expected.
Commenting out the img and/or import, then reverting causes the image asset to be not found.

$ curl http://localhost:9009/assets/logo-ff7cf555f3ab2107e716.png
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8">
<title>Error</title>
</head>
<body>
<pre>Cannot GET /assets/[email protected]</pre>
</body>
</html>

Disabling the cache "fixes" the issue

// webpack.config.js
module.exports = {
  ...
  cache: false,
  ...
}

It's also not an issue when using asset/inline.

webpack.config.js

What is the expected behavior?
I know "it should work" is not helpful, but it seems self explanatory here.

Other relevant information:
webpack version: 5.36.1
Node.js version: 10.24.1
Operating System: OS X 10.14.6
Additional tools:

@sokra
Copy link
Member

sokra commented Apr 30, 2021

Do you have output.clean: true or some other cleaning plugin in config?

@sokra
Copy link
Member

sokra commented Apr 30, 2021

Might be fixed by #13276

@stefan-toubia
Copy link
Author

Yes, I'm using output.clean and confirmed that asset reloading works as expected when clean is disabled. For now I can turn clean off in development.

@sokra
Copy link
Member

sokra commented May 1, 2021

This should be fixed in the latest version

@alexander-akait
Copy link
Member

Confirmed, fixed by https://github.com/webpack/webpack/releases/tag/v5.36.2, the same issue webpack/webpack-dev-server#3175 (comment), let's close, feel free to feedback

@alexilin83
Copy link

It seems that bug still exists. I'm using webpack 5.57.1 and with output.clean = true image assets not fount after hot reload.

@alexander-akait
Copy link
Member

Please provide reproducible test repo

@alexilin83
Copy link

Please provide reproducible test repo

https://github.com/alexilin83/webpack_test_image

@sokra
Copy link
Member

sokra commented Oct 6, 2021

@jantimon This is related to the caching in html-webpack-plugin. output.clean removes all files that are not emitted as compilation.assets. html-webpack-plugin need to make sure to always put their assets into compilation.assets even when cached.

@alexander-akait
Copy link
Member

Duplicate jantimon/html-webpack-plugin#1639

@alexander-akait
Copy link
Member

@alexilin83 Workaround - cache: false for html-webpack-plugin

@alexilin83
Copy link

@alexilin83 Workaround - cache: false for html-webpack-plugin

Doesn't work for me. I updated the test repository.

@alexander-akait
Copy link
Member

hm, weird, looks like it is not real disable cache, anyway we can't fix it here

@alexilin83
Copy link

ok, thanks anyway.

@jantimon
Copy link
Contributor

jantimon commented Oct 6, 2021

@alexander-akait
Copy link
Member

@jantimon here repo https://github.com/alexilin83/webpack_test_image, check this

@alexander-akait
Copy link
Member

No problems without html-webpack-plugin

@alexander-akait
Copy link
Member

@jantimon the main problem - previousEmittedAssets is not contained from HTML template, i.e. <img src="./images/photo.jpg" alt="">, it even do not disabling using cache: false due broken logic for assets, it is really high priory problem

@alexander-akait
Copy link
Member

@jantimon You should emit child assets, go to here https://github.com/jantimon/html-webpack-plugin/blob/main/lib/child-compiler.js#L151, write console.log(childCompilation.assets) and you will see assets which should be reemmited

@alexander-akait
Copy link
Member

To fix this:

in child-compier.js

result[this.templates[entryIndex]] = {
  content: templateSource,
  hash: childCompilation.hash || 'XXXX',
  entry: entries[entryIndex],
  assets: childCompilation.assets,
};

index.js

if (isCompilationCached && options.cache && assetJson === newAssetJson) {
  previousEmittedAssets.forEach(({ name, html, source }) => {
    compilation.emitAsset(name, source ? source : new webpack.sources.RawSource(html, false));
  });
  return callback();
  } else {
    previousEmittedAssets = [];
    assetJson = newAssetJson;
}

// ...
// ...
// ...

const replacedFilename = replacePlaceholdersInFilename(filename, html, compilation);
// Add the evaluated html code to the webpack assets
compilation.emitAsset(replacedFilename.path, new webpack.sources.RawSource(html, false), replacedFilename.info);
previousEmittedAssets.push({ name: replacedFilename.path, html });
              
Object.keys(templateResult.compiledEntry.assets).forEach((name) => {
  const source = templateResult.compiledEntry.assets[name];

  previousEmittedAssets.push({ name: name, source });
})
              
return replacedFilename.path;

Note - it is simple fix and not full, you should keep assetsInfo and re in emitAsset, also it is fix only cache: true, logic is still broken for cache: false, but you can use the same approach (i.e. previousEmittedAssets = [] replace on logic for cache: true), also note - assets can be changed and you should invalidate cache own cache too, the main problem - you should not use custom caching, webpack already has great built-in cache API. Also I don't understand why do you use loader for cache, it is really weird and decrease perf and memory usage, you need just static function (provided by developer) and use it, or you can create own Module (like we do in mini-css-extract-plugin - CssModules) and no problems with cache anymore.

A while ago I asked you to move repo inside webpack-contrib so it will be already implemented and works fine, but for some reason you didn’t want it 🤔

@jantimon
Copy link
Contributor

jantimon commented Oct 7, 2021

thanks alex - sorry I had a long work day yesterday.. let me take a look :)

to summarise the issue to ensure we are on the same page:

  • index.html is depending on './images/photo.jpg
  • ✅ HtmlWebpackPlugin is emitting everything correctly
  • ✅ the user triggers a new compile
  • ✅ webpack clean deletes all assets
  • 🕷 HtmlWebpackPlugin is emitting only .html assets
  • 💣 the image is missing

Can we move this bug to the html-webpack-plugin issue tracker?

Maybe a short note on cache:true/false - the cache option was introduced long long time ago..
back then webpack would emit files again automatically.
basically it does the folloing

  • cache:false will execute the template (think of this.importModule) for every compilation to allow accessing the latest compilation object
  • cache:true will compile and execute only if the html entry file or one of its dependencies change

Caching is a tricky topic and 7 years ago when the html-webpack-plugin was created it was very difficult to have a good performance for child compiler especially without an entry point in webpack

I am happy for better caching ideas - or even better remove them if they are no longer needed..
Could you please try to explain what you mean by "use loader for cache you need just static function by developer"?
Do you mean the templateContent feature?

new HtmlWebpackPlugin({
  templateContent: ({htmlWebpackPlugin}) => `
    <html>
      <head>
        <title>Hello World</title>
      </head>
      <body>
        <h1>Hello World</h1>
      </body>
    </html>
  `
})

one challenge for caching was that some people use the HtmlWebpackPlugin to generate many many many files out of the same template e.g.:

new HtmlWebpackPlugin({
  filename: "[name].html"
})

Do you think we could solve all those cases with an additional Module?
I am very happy if we can find a better caching logic..
It adds a lot of complexity to the html-webpack-plugin

@alexander-akait
Copy link
Member

@jantimon Only you can move this issue to html-webpack-plugin issue tracker, let's do it. You can look how mini-css-extract-plugin implements right logic, it is real not hard

@alexander-akait
Copy link
Member

Fixed in html-webpack-plugin

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants