Skip to content
This repository was archived by the owner on Aug 27, 2026. It is now read-only.

fix: add semicolon to avoid Uncaught TypeError on Webpack v5 - #561

Merged
alexander-akait merged 3 commits into
webpack:masterfrom
abhchand:uncaught-type-error-multiple-files-issue-560
Aug 10, 2020
Merged

alexander-akait merged 3 commits into
webpack:masterfrom
abhchand:uncaught-type-error-multiple-files-issue-560

Conversation

@abhchand

@abhchand abhchand commented Aug 5, 2020

Copy link
Copy Markdown
Contributor

A missing semicolon after the closing } when defining
installedCssChunks causes the object { "common": 0} to be
incorrectly interpreted as a function

This PR contains a:

  • bugfix
  • new feature
  • code refactor
  • test update
  • typo fix
  • metadata update

Motivation / Use-Case

fixes #560

The compiled javascript pack is generated by Webpack v5 as follows:

...

/******/  // object to store loaded CSS chunks
/******/  var installedCssChunks = {
/******/    "common": 0
/******/  }/* webpack/runtime/jsonp chunk loading */
/******/  (() => {
/******/    // object to store loaded and loading chunks
/******/    // undefined = chunk not loaded, null = chunk preloaded/prefetched
/******/    // Promise = chunk loading, 0 = chunk loaded
/******/    var installedChunks = {
/******/      "common": 0
/******/    };

....

Breaking Changes

n/a

Additional Info

See: #560

The compiled javascript pack is generated as follows:

```
...

/******/  // object to store loaded CSS chunks
/******/  var installedCssChunks = {
/******/    "common": 0
/******/  }/* webpack/runtime/jsonp chunk loading */
/******/  (() => {
/******/    // object to store loaded and loading chunks
/******/    // undefined = chunk not loaded, null = chunk preloaded/prefetched
/******/    // Promise = chunk loading, 0 = chunk loaded
/******/    var installedChunks = {
/******/      "common": 0
/******/    };

....
```

A missing semicolon after the closing `}` when defining
`installedCssChunks` causes the object `{ "common": 0}` to be
incorrectly interpreted as a function
@jsf-clabot

jsf-clabot commented Aug 5, 2020

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@codecov

codecov Bot commented Aug 5, 2020

Copy link
Copy Markdown

Codecov Report

Merging #561 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #561   +/-   ##
=======================================
  Coverage   88.55%   88.55%           
=======================================
  Files           5        5           
  Lines         428      428           
  Branches       96       96           
=======================================
  Hits          379      379           
  Misses         47       47           
  Partials        2        2           
Impacted Files Coverage Δ
src/index.js 87.85% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b146549...7000522. Read the comment docs.

@alexander-akait

Copy link
Copy Markdown
Member

Very strange what CI failed 😕 Need investigate

@abhchand

abhchand commented Aug 5, 2020

Copy link
Copy Markdown
Contributor Author

Yeah it seems it's failing for node v10 and webpack@next. I'm seeing 1 failing test when I try this locally on OS X (there seem to be more failures when the CI runs the same thing on Ubutnu though 🤔 )

This test is also failing on the latest master, so it's probably note related to my commit/fix.
More than likely it is something related to webpack@next compatibility.

Unfortunately I'm not sure I know enough about the plugin to easily debug this quickly

● TestCases › hmr should compile to the expected result

    expect(received).toEqual(expected) // deep equality

    - Expected
    + Received

    @@ -1,6 +1,30 @@
      /******/ (() => { // webpackBootstrap
    + /******/ 	// The module cache
    + /******/ 	var __webpack_module_cache__ = {};
    + /******/
    + /******/ 	// The require function
    + /******/ 	function __webpack_require__(moduleId) {
    + /******/ 		// Check if module is in cache
    + /******/ 		if(__webpack_module_cache__[moduleId]) {
    + /******/ 			return __webpack_module_cache__[moduleId].exports;
    + /******/ 		}
    + /******/ 		// Create a new module (and put it into the cache)
    + /******/ 		var module = __webpack_module_cache__[moduleId] = {
    + /******/ 			// no module.id needed
    + /******/ 			// no module.loaded needed
    + /******/ 			exports: {}
    + /******/ 		};
    + /******/
    + /******/ 		// Execute the module function
    + /******/ 		__webpack_modules__[moduleId](module, module.exports, __webpack_require__);
    + /******/
    + /******/ 		// Return the exports of the module
    + /******/ 		return module.exports;
    + /******/ 	}
    + /******/
    + /************************************************************************/
      /******/ 	/************************************************************************/
      // extracted by mini-css-extract-plugin
          if(false) { var cssReload; }
      ··
      /******/ })()

      21 |       const actualContent = fs.readFileSync(path.resolve(actual, file), 'utf8');
      22 |
    > 23 |       expect(actualContent).toEqual(content);
         |                             ^
      24 |     }
      25 |   }
      26 | }

      at compareDirectory (test/TestCases.test.js:23:29)
      at test/TestCases.test.js:99:13
      at compiler.close.err2 (node_modules/webpack/lib/webpack.js:123:6)
      at node_modules/webpack/lib/HookWebpackError.js:69:3
      at Hook.eval [as callAsync] (eval at create (node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:6:1)
      at Hook.CALL_ASYNC_DELEGATE [as _callAsync] (node_modules/tapable/lib/Hook.js:18:14)
      at Cache.shutdown (node_modules/webpack/lib/Cache.js:150:23)
      at Compiler.close (node_modules/webpack/lib/Compiler.js:996:14)
      at compiler.run (node_modules/webpack/lib/webpack.js:122:14)
      at finalCallback (node_modules/webpack/lib/Compiler.js:367:32)
      at cache.storeBuildDependencies.err (node_modules/webpack/lib/Compiler.js:431:17)
      at node_modules/webpack/lib/HookWebpackError.js:69:3
      at Hook.eval [as callAsync] (eval at create (node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:6:1)
      at Hook.CALL_ASYNC_DELEGATE [as _callAsync] (node_modules/tapable/lib/Hook.js:18:14)
      at Cache.storeBuildDependencies (node_modules/webpack/lib/Cache.js:122:37)
      at hooks.done.callAsync.err (node_modules/webpack/lib/Compiler.js:427:19)
      at Hook.eval [as callAsync] (eval at create (node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:6:1)
      at Hook.CALL_ASYNC_DELEGATE [as _callAsync] (node_modules/tapable/lib/Hook.js:18:14)
      at emitRecords.err (node_modules/webpack/lib/Compiler.js:424:23)
      at Compiler.emitRecords (node_modules/webpack/lib/Compiler.js:741:39)
      at emitAssets.err (node_modules/webpack/lib/Compiler.js:416:11)
      at hooks.afterEmit.callAsync.err (node_modules/webpack/lib/Compiler.js:723:14)
      at Hook.eval [as callAsync] (eval at create (node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:6:1)
      at Hook.CALL_ASYNC_DELEGATE [as _callAsync] (node_modules/tapable/lib/Hook.js:18:14)
      at asyncLib.forEachLimit.err (node_modules/webpack/lib/Compiler.js:720:27)
      at node_modules/neo-async/async.js:2818:7
      at done (node_modules/neo-async/async.js:3522:9)
      at alreadyWritten (node_modules/webpack/lib/Compiler.js:582:8)
      at outputFileSystem.readFile (node_modules/webpack/lib/Compiler.js:653:19)
      at node_modules/graceful-fs/graceful-fs.js:123:16

@alexander-akait

Copy link
Copy Markdown
Member

Can you update snapshot?

@abhchand

abhchand commented Aug 6, 2020

Copy link
Copy Markdown
Contributor Author

@evilebottnawi, @sokra - I added a new commit (bf2de3d) to update the snapshot for the failing test (test/cases/hmr/expected/webpack-5/main.js).

Everything passes for me locally now with webpack 5.

Screen Shot 2020-08-06 at 11 41 30 AM

The CI is still failing for unrelated tests, and it seems to be because of a setup/configuration issue with the test directories. These are probably outside of the scope of this fix, right? Example:

 ● TestCases › js-hash should compile to the expected result

    ENOENT: no such file or directory, open '/home/runner/work/mini-css-extract-plugin/mini-css-extract-plugin/test/js/js-hash/style.922798e08e96756adb4a.1.css'

      19 |     } else if (stats.isFile()) {
      20 |       const content = fs.readFileSync(path.resolve(expected, file), 'utf8');
    > 21 |       const actualContent = fs.readFileSync(path.resolve(actual, file), 'utf8');
         |                                ^
      22 | 
      23 |       expect(actualContent).toEqual(content);
      24 |     }

      at compareDirectory (test/TestCases.test.js:21:32)
      at test/TestCases.test.js:104:13
      at compiler.close.err2 (node_modules/webpack/lib/webpack.js:123:6)
      at node_modules/neo-async/async.js:2830:7
      at done (node_modules/neo-async/async.js:2865:11)
      at node_modules/neo-async/async.js:2818:7
      at node_modules/webpack/lib/HookWebpackError.js:69:3
      at Hook.eval [as callAsync] (eval at create (node_modules/webpack/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:6:1)

Thanks again!

@alexander-akait

Copy link
Copy Markdown
Member

Yes, it is outside, I will look at this tomorrow and we can do patch release

@alexander-akait

Copy link
Copy Markdown
Member

@abhchand I think we need to add tests to avoid future regressions, can you do it? Just simple test (only for webpack@5)

@abhchand

abhchand commented Aug 8, 2020

Copy link
Copy Markdown
Contributor Author

@evilebottnawi - I'll have to figure out how the tests work, so it might take me a few days. But sure, I'll give it a try.

Adding 2 tests:

* `dependOn` - Checks that `dependOn` feature works
* `dependOn-multiple-files-per-entry` - Webpack 5 supports specifying multiple files per entry point in combination with the `dependOn` key

Motivated by the issue described in
#560
@abhchand

abhchand commented Aug 9, 2020

Copy link
Copy Markdown
Contributor Author

@evilebottnawi - I added 2 tests for the dependOn functionality that Webpack supports (7000522), since that's what caused the semi-colon error above.

1. dependOn

This tests that the dependOn keyword works correctly with MiniCSSExtractPlugin

  entry: {
    entry1: { import: './entryA.js', dependOn: 'common' },
    common: './entryB.js',
  },

2. dependOn-multiple-files-per-entry

This tests the issue that I opened (#560) related to how dependOn behaves with multiple files per entry, which is newly supported in Webpack 5.

  entry: {
    entry1: { import: ['./entryA.js', './entryB.js'], dependOn: 'common' },
    common: ['./entryC.js', './entryD.js'],
  },

Problem

I ran into one issue that you might know how to fix:

Both of these tests pass on Webpack 5, but they fail on Webpack 4 (on my local machine) because the webpack.config.js fails validation.

How do I make theses tests run only against webpack5?

Thanks.

@alexander-akait

Copy link
Copy Markdown
Member

I think we can add filter.js file in each directory, where return true means run test return false means not run test, so we just will not run tests for webpack@5 on webpack@4

@alexander-akait

Copy link
Copy Markdown
Member

But i think it is out work, not you, I will merge it as is and fix tests in near future (today)

@alexander-akait

alexander-akait commented Aug 10, 2020

Copy link
Copy Markdown
Member

WIP on tests, thanks for the PR

@alexander-akait
alexander-akait merged commit 3974210 into webpack:master Aug 10, 2020
@abhchand

Copy link
Copy Markdown
Contributor Author

@evilebottnawi - Спасибо за вашу помощь :)

@alexander-akait

Copy link
Copy Markdown
Member

@abhchand Thanks for using webpack

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.

Uncaught TypeError when using multiple files per entry on Webpack v5

3 participants