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 asset cache with "extract-text-webpack-plugin" #3806

Closed
wants to merge 5 commits into from

Conversation

yuriyostapenko
Copy link
Contributor

@yuriyostapenko yuriyostapenko commented Jan 6, 2017

What kind of change does this PR introduce?
bugfix

Did you add tests for your changes?
yes

If relevant, link to documentation update:

Summary
This is related to a fix in webpack-contrib/extract-text-webpack-plugin#325, adds test to changes made in that PR and will fix #3744 in a more reliable manner.

Does this PR introduce a breaking change?
no

Other information
Add two tests asserting watch cache behavior (with and without the plugin).
The lack of extra asset caching causes webpack to emit css files on every change (and report build taking place with noop JS changes).

Add support for skipping watch test steps when no build is expected.

Revert actual change from #3744

Now that "extract-text-webpack-plugin" correctly adds asset hashes to compilation hash, this can be removed.

I'm actually not sure whether it makes sense to keep it (not aware of any other plugins affected by this),
But I'd remove it at least for now, because of how extract-text-webpack-plugin works today.
Child compilation is only added if css changes, which breaks test case watchCases/plugins/extract-text-plugin step 3 (no-op JS change) on Windows and Linux.
It works on macOS though, but only as a result of another bug: watchpack reports all files as changed on that platform.

Yuriy Ostapenko added 3 commits January 6, 2017 10:23
Add two tests asserting watch cache behavior (with and without the plugin).
The lack of extra asset caching causes webpack to emit css files on every change (and report build taking place with noop JS changes).

Add support for skipping watch test steps when no build is expected.
Now that "extract-text-webpack-plugin" correctly adds asset hashes to compilation hash, this can be removed.

I'm actually not sure whether it makes sense to keep it (not aware of any other plugins affected by this),
But I'd remove it at least for now, because of how extract-text-webpack-plugin works today.
Child compilation is only added if css changes, which breaks test case watchCases/plugins/extract-text-plugin step 3 (no-op JS change) on Windows and Linux.
It works on macOS though, but only as a result of another bug: watchpack reports all files as changed on that platform.
@yuriyostapenko yuriyostapenko changed the title Fix asset cache Fix asset cache with "extract-text-webpack-plugin" Jan 6, 2017
@yuriyostapenko
Copy link
Contributor Author

yuriyostapenko commented Jan 6, 2017

I'd need some help or advice resolving the "chicken and egg" problem between this and webpack-contrib/extract-text-webpack-plugin#325:

Tests added in this PR won't work unless extract-text-webpack-plugin is updated.

@sokra, @TheLarkInn

Excuse: beautify-lint does not match file patterns with forward slash on Windows
@yuriyostapenko
Copy link
Contributor Author

Hi @sokra,
Sorry to bother, but I'd really appreciate some feedback on this when you have time.
I'm sure the test assertions are valid, but would like to know if you agree with direction in the fix (or should handling non-cache-aware extra assets be implemented in the core maybe?)

@webpack-bot
Copy link
Contributor

@sokra Please review the following output log for errors:

  4 failing

  1) Stats should print correct stats for separate-css-bundle:

      Uncaught AssertionError: expected 'Hash: 0be4035986816982617a1139e89514abc13454ce\nChild\n    Hash: 0be4035986816982617a\n    Time: Xms\n                                   Asset      Size  Chunks             Chunk Names\n                 c7ab11336573e45dc51e.js   2.77 kB       0  [emitted]  main\n    c815cf440254d4f3bba4e7041db00a28.css  26 bytes       0  [emitted]  main\n    chunk    {0} c7ab11336573e45dc51e.js, c815cf440254d4f3bba4e7041db00a28.css (main) 64 bytes [entry] [rendered]\n        [0] (webpack)/test/statsCases/separate-css-bundle/a/file.css 41 bytes {0} [built]\n        [1] (webpack)/test/statsCases/separate-css-bundle/a/index.js 23 bytes {0} [built]\n    Child extract-text-webpack-plugin:\n        chunk    {0} extract-text-webpack-plugin-output-filename 1.65 kB [entry] [rendered]\n            [0] (webpack)/~/css-loader!(webpack)/test/statsCases/separate-css-bundle/a/file.css 190 bytes {0} [built]\n            [1] (webpack)/~/css-loader/lib/css-base.js 1.46 kB {0} [built]\nChild\n    Hash: 1139e89514abc13454ce\n    Time: Xms\n                                   Asset      Size  Chunks             Chunk Names\n                 c7ab11336573e45dc51e.js   2.77 kB       0  [emitted]  main\n    a3f385680aef7a9bb2a517699532cc34.css  28 bytes       0  [emitted]  main\n    chunk    {0} c7ab11336573e45dc51e.js, a3f385680aef7a9bb2a517699532cc34.css (main) 64 bytes [entry] [rendered]\n        [0] (webpack)/test/statsCases/separate-css-bundle/b/file.css 41 bytes {0} [built]\n        [1] (webpack)/test/statsCases/separate-css-bundle/b/index.js 23 bytes {0} [built]\n    Child extract-text-webpack-plugin:\n        chunk    {0} extract-text-webpack-plugin-output-filename 1.65 kB [entry] [rendered]\n            [0] (webpack)/~/css-loader!(webpack)/test/statsCases/separate-css-bundle/b/file.css 192 bytes {0} [built]\n            [1] (webpack)/~/css-loader/lib/css-base.js 1.46 kB {0} [built]' to equal 'Hash: 3a94b16fac8bc7176bab0eec561d85889150f958\nChild\n    Hash: 3a94b16fac8bc7176bab\n    Time: Xms\n                                   Asset      Size  Chunks             Chunk Names\n                 c7ab11336573e45dc51e.js   2.77 kB       0  [emitted]  main\n    c815cf440254d4f3bba4e7041db00a28.css  26 bytes       0  [emitted]  main\n    chunk    {0} c7ab11336573e45dc51e.js, c815cf440254d4f3bba4e7041db00a28.css (main) 64 bytes [entry] [rendered]\n        [0] (webpack)/test/statsCases/separate-css-bundle/a/file.css 41 bytes {0} [built]\n        [1] (webpack)/test/statsCases/separate-css-bundle/a/index.js 23 bytes {0} [built]\n    Child extract-text-webpack-plugin:\n        chunk    {0} extract-text-webpack-plugin-output-filename 1.65 kB [entry] [rendered]\n            [0] (webpack)/~/css-loader!(webpack)/test/statsCases/separate-css-bundle/a/file.css 190 bytes {0} [built]\n            [1] (webpack)/~/css-loader/lib/css-base.js 1.46 kB {0} [built]\nChild\n    Hash: 0eec561d85889150f958\n    Time: Xms\n                                   Asset      Size  Chunks             Chunk Names\n                 c7ab11336573e45dc51e.js   2.77 kB       0  [emitted]  main\n    a3f385680aef7a9bb2a517699532cc34.css  28 bytes       0  [emitted]  main\n    chunk    {0} c7ab11336573e45dc51e.js, a3f385680aef7a9bb2a517699532cc34.css (main) 64 bytes [entry] [rendered]\n        [0] (webpack)/test/statsCases/separate-css-bundle/b/file.css 41 bytes {0} [built]\n        [1] (webpack)/test/statsCases/separate-css-bundle/b/index.js 23 bytes {0} [built]\n    Child extract-text-webpack-plugin:\n        chunk    {0} extract-text-webpack-plugin-output-filename 1.65 kB [entry] [rendered]\n            [0] (webpack)/~/css-loader!(webpack)/test/statsCases/separate-css-bundle/b/file.css 192 bytes {0} [built]\n            [1] (webpack)/~/css-loader/lib/css-base.js 1.46 kB {0} [built]'
      + expected - actual

      -Hash: 0be4035986816982617a1139e89514abc13454ce
      +Hash: 3a94b16fac8bc7176bab0eec561d85889150f958
       Child
      -    Hash: 0be4035986816982617a
      +    Hash: 3a94b16fac8bc7176bab
           Time: Xms
                                          Asset      Size  Chunks             Chunk Names
                        c7ab11336573e45dc51e.js   2.77 kB       0  [emitted]  main
           c815cf440254d4f3bba4e7041db00a28.css  26 bytes       0  [emitted]  main
               chunk    {0} extract-text-webpack-plugin-output-filename 1.65 kB [entry] [rendered]
                   [0] (webpack)/~/css-loader!(webpack)/test/statsCases/separate-css-bundle/a/file.css 190 bytes {0} [built]
                   [1] (webpack)/~/css-loader/lib/css-base.js 1.46 kB {0} [built]
       Child
      -    Hash: 1139e89514abc13454ce
      +    Hash: 0eec561d85889150f958
           Time: Xms
                                          Asset      Size  Chunks             Chunk Names
                        c7ab11336573e45dc51e.js   2.77 kB       0  [emitted]  main
           a3f385680aef7a9bb2a517699532cc34.css  28 bytes       0  [emitted]  main
      
      at Assertion.value (/Users/travis/build/webpack/webpack/test/js/config/extract-text/issue-14/bundle0.js:2080:19)
      at c.run (/Users/travis/build/webpack/webpack/test/Stats.test.js:98:22)
      at /Users/travis/build/webpack/webpack/lib/MultiCompiler.js:9:10571
      at /Users/travis/build/webpack/webpack/node_modules/async/dist/async.js:1012:9
      at /Users/travis/build/webpack/webpack/node_modules/async/dist/async.js:359:16
      at iteratorCallback (/Users/travis/build/webpack/webpack/node_modules/async/dist/async.js:935:13)
      at /Users/travis/build/webpack/webpack/node_modules/async/dist/async.js:843:16
      at /Users/travis/build/webpack/webpack/node_modules/async/dist/async.js:1009:13
      at runCompilers (/Users/travis/build/webpack/webpack/lib/MultiCompiler.js:9:5862)
      at /Users/travis/build/webpack/webpack/lib/MultiCompiler.js:9:6476
      at /Users/travis/build/webpack/webpack/lib/MultiCompiler.js:9:10251
      at /Users/travis/build/webpack/webpack/lib/Compiler.js:9:18695
      at Compiler.emitRecords (/Users/travis/build/webpack/webpack/lib/Compiler.js:9:24807)
      at /Users/travis/build/webpack/webpack/lib/Compiler.js:9:18097
      at /Users/travis/build/webpack/webpack/lib/Compiler.js:9:24492
      at next (/Users/travis/build/webpack/webpack/node_modules/tapable/lib/Tapable.js:154:11)
      at Compiler.compiler.plugin (/Users/travis/build/webpack/webpack/lib/performance/SizeLimitsPlugin.js:9:4844)
      at Compiler.applyPluginsAsyncSeries1 (/Users/travis/build/webpack/webpack/node_modules/tapable/lib/Tapable.js:158:13)
      at Compiler.afterEmit (/Users/travis/build/webpack/webpack/lib/Compiler.js:9:24142)
      at Compiler.<anonymous> (/Users/travis/build/webpack/webpack/lib/Compiler.js:9:24011)
      at /Users/travis/build/webpack/webpack/node_modules/async/dist/async.js:359:16
      at iteratorCallback (/Users/travis/build/webpack/webpack/node_modules/async/dist/async.js:935:13)
      at /Users/travis/build/webpack/webpack/node_modules/async/dist/async.js:843:16
      at /Users/travis/build/webpack/webpack/node_modules/graceful-fs/graceful-fs.js:43:10
  

  2) WatchTestCases context delete-in-context should compile:
     Uncaught Error: ENOENT: no such file or directory, unlink '/Users/travis/build/webpack/webpack/test/js/watch-src/context/delete-in-context/directory/b.js'
      at files.forEach (/Users/travis/build/webpack/webpack/test/WatchTestCases.test.js:26:8)
      at Array.forEach (native)
      at copyDiff (/Users/travis/build/webpack/webpack/test/WatchTestCases.test.js:17:8)
      at files.forEach (/Users/travis/build/webpack/webpack/test/WatchTestCases.test.js:22:4)
      at Array.forEach (native)
      at copyDiff (/Users/travis/build/webpack/webpack/test/WatchTestCases.test.js:17:8)
      at Timeout._onTimeout (/Users/travis/build/webpack/webpack/test/WatchTestCases.test.js:186:11)
      at ontimeout (timers.js:386:14)
      at tryOnTimeout (timers.js:250:5)
      at Timer.listOnTimeout (timers.js:214:5)
  

  3) WatchTestCases context delete-in-context 2 should detect changes in a context:

      AssertionError: expected 2 to equal 4
      + expected - actual

      -2
      +4
      
      at p.value (/Users/travis/build/webpack/webpack/test/js/all-combined/runtime/module-caching/bundle.js:19:3807)
      at Context.<anonymous> (/Users/travis/build/webpack/webpack/test/js/watch/context/delete-in-context/bundle.js:109:34)
      at callFn (/Users/travis/build/webpack/webpack/node_modules/mocha/lib/runnable.js:345:21)
      at Test.Runnable.run (/Users/travis/build/webpack/webpack/node_modules/mocha/lib/runnable.js:337:7)
      at Runner.runTest (/Users/travis/build/webpack/webpack/node_modules/mocha/lib/runner.js:444:10)
      at /Users/travis/build/webpack/webpack/node_modules/mocha/lib/runner.js:550:12
      at /Users/travis/build/webpack/webpack/node_modules/mocha/lib/runner.js:371:7
      at Immediate.<anonymous> (/Users/travis/build/webpack/webpack/node_modules/mocha/lib/runner.js:339:5)
      at tryOnImmediate (timers.js:645:5)
      at processImmediate [as _immediateCallback] (timers.js:617:5)
  

  4) WatchTestCases parsing caching-harmony should compile:
     Uncaught Error: ENOENT: no such file or directory, unlink '/Users/travis/build/webpack/webpack/test/js/watch-src/context/delete-in-context/directory/a.js'
      at files.forEach (/Users/travis/build/webpack/webpack/test/WatchTestCases.test.js:26:8)
      at Array.forEach (native)
      at copyDiff (/Users/travis/build/webpack/webpack/test/WatchTestCases.test.js:17:8)
      at files.forEach (/Users/travis/build/webpack/webpack/test/WatchTestCases.test.js:22:4)
      at Array.forEach (native)
      at copyDiff (/Users/travis/build/webpack/webpack/test/WatchTestCases.test.js:17:8)
      at Timeout._onTimeout (/Users/travis/build/webpack/webpack/test/WatchTestCases.test.js:186:11)
      at ontimeout (timers.js:386:14)
      at tryOnTimeout (timers.js:250:5)
      at Timer.listOnTimeout (timers.js:214:5)
  

See complete report here.

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

Successfully merging this pull request may close these issues.

None yet

3 participants