webpack / watchpack

Wrapper library for directory and file watching.
MIT License
377 stars 105 forks source link

Watchpack.watch() should not accept undefined #217 #218

Closed basuke closed 2 years ago

basuke commented 2 years ago

https://github.com/webpack/watchpack/issues/217

What happened

Webpack crashes when watch() got a undefined as a function. This is a typical example:

$ webpack --watch --mode=development
node:internal/validators:119
    throw new ERR_INVALID_ARG_TYPE(name, 'string', value);
    ^

TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string. Received undefined
    at new NodeError (node:internal/errors:363:5)
    at validateString (node:internal/validators:119:11)
    at Object.dirname (node:path:1276:5)
    at WatcherManager.watchFile (/Users/basuke/work/assiston/site/node_modules/watchpack/lib/getWatcherManager.js:30:26)
    at /Users/basuke/work/assiston/site/node_modules/watchpack/lib/watchpack.js:278:41
    at Object.exports.batch (/Users/basuke/work/assiston/site/node_modules/watchpack/lib/watchEventSource.js:326:3)
    at Watchpack.watch (/Users/basuke/work/assiston/site/node_modules/watchpack/lib/watchpack.js:276:20)
    at NodeWatchFileSystem.watch (/Users/basuke/work/assiston/site/node_modules/webpack/lib/node/NodeWatchFileSystem.js:107:16)
    at Watching.watch (/Users/basuke/work/assiston/site/node_modules/webpack/lib/Watching.js:325:48)
    at /Users/basuke/work/assiston/site/node_modules/webpack/lib/Watching.js:303:13 {
  code: 'ERR_INVALID_ARG_TYPE'
}
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

m1-air in work/assiston/site on feature/bye-elm

Source of the issue

Fundamentally it is a bug of the library client, which passes undefined to watch(), but watchpack is so popular to be used as part of webpack, it is practical to solve the issue by protecting the function in watch().

linux-foundation-easycla[bot] commented 2 years ago

CLA Signed

The committers listed above are authorized under a signed CLA.

basuke commented 2 years ago

In the webpack, this files are typed as LazySet<string>. The other values should not be passed. Current behavior is already doing the right thing which throws TypeError. No need to add extra check.