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

Timestamp based check for changes to HTML template #965

Closed
wants to merge 1 commit into from

Conversation

dwoznicki
Copy link

@dwoznicki dwoznicki commented Jun 3, 2018

As of webpack version 4, isCompilationCached no longer appears to be a reliable method of checking whether a template file has been changed. Instead, we can check compilation.fileTimestamps to see when the template file was last modified.
For a 25 html page project, I've found this change reduces the rebuild time from ~3000ms to ~700ms in --watch mode.

NOTE: this commit fails the build test because WebpackRecompilationSimulator.simulateFileChange does not change the compilation.fileTimestamps object.
One way to solve this might be to have simulateFileChange write to the source file and then immediately revert the changes. Another might be to initialize the HtmlWebpackPlugin with the temp file as the template instead of the source file. Both require making changes to webpack-recompilation-simulator, which I'm not sure how to build.
In practice, I've found this commit to work as expected with webpack --watch.

…r whether html file should be rebuilt

Move rebuild check logic into needsRebuild() function
@jantimon
Copy link
Owner

jantimon commented Jun 3, 2018

Wow cool :)

Actually the my webpack-recompilation-simulator will really change a file:

https://github.com/jantimon/webpack-recompilation-simulator/blob/ae4e111f41124f292327ce716e4f145c7f898e54/src/index.js#L34-L51

If there is a bug in the webpack-recompilation-simulator we would have to fix it first.. otherwise it will be really hard to test caching.

There is a little bit more todo for your timestamp logic - right now you are only checking for the timestamp of the template file but not for the timestamp of its dependencies.

Also we should try to prepare your change for #953

@dwoznicki
Copy link
Author

I actually have a solution to check for changes to dependencies, but it causes the “HtmlWebpackPluginCaching won’t recompile if only a javascript file was changed” test to fail. Do we really want the html file to rebuild when a js dependency is changed? I guess so to change the options.hash hash. Anyway, I’ll make another commit later.

@jantimon
Copy link
Owner

jantimon commented Jun 3, 2018

Right now I am fiddling around with a new child compiler but I need some more time to get it working.

@jantimon
Copy link
Owner

jantimon commented Jun 3, 2018

@dwoznicki I just opened #967 which fixes the first part of the issue.

Right now it will revoke the cache in the emit phase:

childCompiler.clearCache(compiler);

Instead we should store the compilation dependencies during the child compiler compilation and check in the compile function if it needs to rebuild.

I also added an array which holds all files used for the template generation:
da2d211

What do you think?

@dwoznicki
Copy link
Author

Cool! That should work quite well. I'm going to grab your changes in feature/single-compiler and make another pull request actually since it strikes me that I'm attempting to merge into master instead of webpack-4 here.

@dwoznicki dwoznicki closed this Jun 8, 2018
@lock
Copy link

lock bot commented Jul 8, 2018

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jul 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants