fix: improve schema for watch options by snitin315 · Pull Request #4424 · webpack/webpack-dev-server

@snitin315

  • This is a bugfix
  • This is a feature
  • This is a code refactor
  • This is a test update
  • This is a docs update
  • This is a metadata update

For Bugs and Features; did you add new tests?

Yes.

Motivation / Use-Case

Resolve #4362

Screen.Recording.2022-05-03.at.7.29.34.AM.mov

Breaking Changes

No

Additional Info

No

alexander-akait

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm, so chokidar doesn't validate own options?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, I think we allow some extra options for watching, but chokidar will only understand its API options.

if (typeof watchOptions.poll !== "undefined") {
return Boolean(watchOptions.poll);
}
if (typeof compilerWatchOptions.poll !== "undefined") {
return Boolean(compilerWatchOptions.poll);
}
// TODO: we respect these options for all watch options and allow developers to pass them to chokidar, but chokidar doesn't have these options maybe we need revisit that in future

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

weird, there are problem when new options will be added, in this case we will need to add them too, it is not good

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, ideally chokidar should have its own validation. Maybe we can add an option in scema-utils additionalProperties: "warn" which will warn if you use another property instead of throwing an error and exiting the process.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds better, we will still needed update them when new options were added, but it happens rarely

@snitin315

@snitin315

@codecov