Skip to content

feat(compiler): Use timestamps to verify cache validity #972

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

Merged
merged 4 commits into from
Jul 11, 2018

Conversation

jantimon
Copy link
Owner

@jantimon jantimon commented Jun 8, 2018

Based on #967 - try to im prove the performance by caching the entire child compilation.

This chang verifies the cache by comparing the child compilations file dependencies with the main compilation timestamps.

@jantimon
Copy link
Owner Author

jantimon commented Jun 8, 2018

all tests pass for this branch for me - could some please please try it on their machine?

@edmorley
Copy link
Contributor

edmorley commented Jun 8, 2018

@jantimon travis is failing?

@dwoznicki
Copy link

dwoznicki commented Jun 8, 2018

Mmm, tests are not passing on my machine. Let me see if I can figure out why.

Sorry I didn't get a PR for this in. I've actually been working on it, but running into some bizarre bugs (something with file watchers being removed after the first compilation??).

Edit: Okay, the same tests that fail for travis are failing for me it seems. My guess is that it has something to do with simulateFileChange not properly updating the file timestamp. I'll look into it after work today.

@dwoznicki
Copy link

dwoznicki commented Jun 9, 2018

By the way, here's what I've found so far.

HtmlWebpackPluginCaching should keep watching the webpack html if only a js file was changed fails because the plugin appears to be compiling html whenever any file is changed. I have a solution for this one, though it'll probably involve changing webpack-recompilation-simulator.

HtmlWebpackPluginCaching should compile the webpack html if the template file was changed fails due to a childCompilation error. The test appears to be unable to correctly resolve the path to lodash.js for me, but I'm having trouble figuring out why. The stack trace looks like so:

[ ModuleNotFoundError: Module not found: Error: Can't resolve '../../home/danwoz/html-webpack-plugin/node_modules/lodash/lodash.js' in '/home/danwoz/html-webpack-plugin/spec/fixtures'
    at factory.create (/home/danwoz/html-webpack-plugin/node_modules/webpack/lib/Compilation.js:522:10)
    at factory (/home/danwoz/html-webpack-plugin/node_modules/webpack/lib/NormalModuleFactory.js:358:22)
    at resolver (/home/danwoz/html-webpack-plugin/node_modules/webpack/lib/NormalModuleFactory.js:118:21)
    at asyncLib.parallel (/home/danwoz/html-webpack-plugin/node_modules/webpack/lib/NormalModuleFactory.js:198:22)
    at /home/danwoz/html-webpack-plugin/node_modules/neo-async/async.js:2817:7
    at /home/danwoz/html-webpack-plugin/node_modules/neo-async/async.js:6783:13
    at normalResolver.resolve (/home/danwoz/html-webpack-plugin/node_modules/webpack/lib/NormalModuleFactory.js:188:25)
    at doResolve (/home/danwoz/html-webpack-plugin/node_modules/webpack/node_modules/enhanced-resolve/lib/Resolver.js:181:12)
    at hook.callAsync (/home/danwoz/html-webpack-plugin/node_modules/webpack/node_modules/enhanced-resolve/lib/Resolver.js:232:5)
    at _fn0 (eval at create (/home/danwoz/html-webpack-plugin/node_modules/tapable/lib/HookCodeFactory.js:24:12), <anonymous>:15:1)
resolve '../../home/danwoz/html-webpack-plugin/node_modules/lodash/lodash.js' in '/home/danwoz/html-webpack-plugin/spec/fixtures'
  using description file: /home/danwoz/html-webpack-plugin/package.json (relative path: ./spec/fixtures)
    Field 'browser' doesn't contain a valid alias configuration
    using description file: /home/danwoz/html-webpack-plugin/package.json (relative path: ./home/danwoz/html-webpack-plugin/node_modules/lodash/lodash.js)
      no extension
        Field 'browser' doesn't contain a valid alias configuration
        /home/danwoz/html-webpack-plugin/home/danwoz/html-webpack-plugin/node_modules/lodash/lodash.js doesn't exist
      .wasm
        Field 'browser' doesn't contain a valid alias configuration
        /home/danwoz/html-webpack-plugin/home/danwoz/html-webpack-plugin/node_modules/lodash/lodash.js.wasm doesn't exist
      .mjs
        Field 'browser' doesn't contain a valid alias configuration
        /home/danwoz/html-webpack-plugin/home/danwoz/html-webpack-plugin/node_modules/lodash/lodash.js.mjs doesn't exist
      .js
        Field 'browser' doesn't contain a valid alias configuration
        /home/danwoz/html-webpack-plugin/home/danwoz/html-webpack-plugin/node_modules/lodash/lodash.js.js doesn't exist
      .json
        Field 'browser' doesn't contain a valid alias configuration
        /home/danwoz/html-webpack-plugin/home/danwoz/html-webpack-plugin/node_modules/lodash/lodash.js.json doesn't exist
      as directory
        /home/danwoz/html-webpack-plugin/home/danwoz/html-webpack-plugin/node_modules/lodash/lodash.js doesn't exist ]

For reference, the html-webpack-plugin base directory is /home/danwoz/html-webpack-plugin, and I'd expect lodash to resolve to /home/danwoz/html-webpack-plugin/node_modules/lodash/lodash.js.

Any ideas?

@jantimon jantimon changed the base branch from feature/single-compiler to webpack-4 June 21, 2018 07:48
@jantimon
Copy link
Owner Author

@denis-sokolov "HtmlWebpackPluginCaching should keep watching the webpack html if only a js file was changed fails because the plugin appears to be compiling html whenever any file is changed. I have a solution for this one, though it'll probably involve changing webpack-recompilation-simulator."

Would it be possible to add a test for that case?

@jantimon jantimon force-pushed the feature/cache-file-timestamps branch from fbc439c to 5d06fd0 Compare June 21, 2018 07:53
@denis-sokolov

This comment has been minimized.

@jantimon

This comment has been minimized.

@linkenneth
Copy link

Thanks for the amazing work here and at #967. Any estimates on how much of an improvement this will be?

@jantimon
Copy link
Owner Author

@linkenneth the performance gain depends on your webpack version, project size and what you do with the way how you use the html-webpack-plugin

@jantimon
Copy link
Owner Author

@dwoznicki I could find the reason for the childCompilation error with the unresolved dependency:

webpack/loader-utils#115

@jantimon jantimon force-pushed the feature/cache-file-timestamps branch from 5d06fd0 to 539b34b Compare July 4, 2018 09:14
@jantimon
Copy link
Owner Author

jantimon commented Jul 4, 2018

@dwoznicki All previous tests pass!!

I have also added a new test which uses the new watchMode features of the webpack-recompilation-simulator and could verify the bug you already meantioned.

I will try to port over your addDependencies solution.

@emilio-martinez
Copy link

Tested this PR. It resolves the issues I had with --watch and dev-server on #953. Good work! And thank you! 👏👏👏

@dwoznicki
Copy link

For reference, my project (which has grown substantially since #962) builds in ~8000ms without and ~1000ms with these changes.

@lock
Copy link

lock bot commented Aug 30, 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 Aug 30, 2018
@jantimon jantimon deleted the feature/cache-file-timestamps branch January 2, 2020 11:21
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.

6 participants