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.

Sign up for GitHub

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

Jump to bottom

fix: improve schema for watch options #4424

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+156 −22
Open

fix: improve schema for watch options #4424

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a01150b
fix: improve schema for watch options
snitin315 May 3, 2022
ae2312b
fix: update types
snitin315 May 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
67 changes: 60 additions & 7 deletions lib/options.json
View file
Open in desktop
Expand Up @@ -1002,9 +1002,7 @@
}
},
{
"type": "object",
"description": "Options for watch.",
"link": "https://github.com/paulmillr/chokidar#api"
"$ref": "#/definitions/WatchOptionsChokidar"
}
],
"description": "Watches for files in static content directory.",
Expand Down Expand Up @@ -1064,10 +1062,7 @@
"description": "Path(s) of globs/directories/files to watch for file changes."
},
"options": {
"type": "object",
"description": "Configure advanced options for watching. See the chokidar documentation for the possible options.",
"link": "https://github.com/paulmillr/chokidar#api",
"additionalProperties": true
"$ref": "#/definitions/WatchOptionsChokidar"
}
},
"additionalProperties": false
Expand All @@ -1076,6 +1071,64 @@
"type": "string",
"minLength": 1
},
"WatchOptionsChokidar": {
"type": "object",
"additionalProperties": false,
"description": "Configure advanced options for watching. See the chokidar documentation for the possible options.",
"link": "https://github.com/paulmillr/chokidar#api",
"properties": {
"persistent": {
"type": "boolean"
},
"ignored": {
"type": "string"
},
"ignoreInitial": {
"type": "boolean"
},
"followSymlinks": {
"type": "boolean"
},
"cwd": {
"type": "string"
},
"disableGlobbing": {
"type": "boolean"
},
"usePolling": {
"type": "boolean"
},
"interval": {
"type": "number"
},
"binaryInterval": {
"type": "number"
},
"alwaysStat": {
"type": "boolean"
},
"depth": {
"type": "number"
},
"awaitWriteFinish": {
"type": "object",
"additionalProperties": true
},
"ignorePermissionErrors": {
"type": "boolean"
},
"atomic": {
"anyOf": [
{
"type": "boolean"
},
{
"type": "number"
}
]
}
}
},
Copy link
Member

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?

Copy link
Member Author

Choose a reason for hiding this comment

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

@alexander-akait No, it doesn't.

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

Copy link
Member Author

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.

webpack-dev-server/lib/Server.js

Lines 766 to 772 in c2fee3b

if (typeof watchOptions.poll !== "undefined") {
return Boolean(watchOptions.poll);
}
if (typeof compilerWatchOptions.poll !== "undefined") {
return Boolean(compilerWatchOptions.poll);
}

webpack-dev-server/lib/Server.js

Line 805 in c2fee3b

// 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

Copy link
Member

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

Copy link
Member Author

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.

Copy link
Member

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

"WebSocketServer": {
"anyOf": [
{
Expand Down
24 changes: 20 additions & 4 deletions test/__snapshots__/validate-options.test.js.snap.webpack5
View file
Open in desktop
Expand Up @@ -781,6 +781,14 @@ exports[`options validate should throw an error on the "static" option with '{"s
object { … }"
`;

exports[`options validate should throw an error on the "static" option with '{"watch":{"invalid":true}}' value 1`] = `
"ValidationError: Invalid options object. Dev Server has been initialized using an options object that does not match the API schema.
- options.static.watch has an unknown property 'invalid'. These properties are valid:
object { persistent?, ignored?, ignoreInitial?, followSymlinks?, cwd?, disableGlobbing?, usePolling?, interval?, binaryInterval?, alwaysStat?, depth?, awaitWriteFinish?, ignorePermissionErrors?, atomic? }
-> Configure advanced options for watching. See the chokidar documentation for the possible options.
-> Read more at https://github.com/paulmillr/chokidar#api"
`;

exports[`options validate should throw an error on the "static" option with '{"watch":10}' value 1`] = `
"ValidationError: Invalid options object. Dev Server has been initialized using an options object that does not match the API schema.
- options.static should be one of these:
Expand All @@ -789,14 +797,14 @@ exports[`options validate should throw an error on the "static" option with '{"w
-> Read more at https://webpack.js.org/configuration/dev-server/#devserverstatic
Details:
* options.static.watch should be one of these:
boolean | object { … }
boolean | object { persistent?, ignored?, ignoreInitial?, followSymlinks?, cwd?, disableGlobbing?, usePolling?, interval?, binaryInterval?, alwaysStat?, depth?, awaitWriteFinish?, ignorePermissionErrors?, atomic? }
-> Watches for files in static content directory.
-> Read more at https://webpack.js.org/configuration/dev-server/#watch
Details:
* options.static.watch should be a boolean.
* options.static.watch should be an object:
object { … }
-> Options for watch.
object { persistent?, ignored?, ignoreInitial?, followSymlinks?, cwd?, disableGlobbing?, usePolling?, interval?, binaryInterval?, alwaysStat?, depth?, awaitWriteFinish?, ignorePermissionErrors?, atomic? }
-> Configure advanced options for watching. See the chokidar documentation for the possible options.
-> Read more at https://github.com/paulmillr/chokidar#api"
`;

Expand Down Expand Up @@ -830,10 +838,18 @@ exports[`options validate should throw an error on the "static" option with 'nul
object { directory?, staticOptions?, publicPath?, serveIndex?, watch? }"
`;

exports[`options validate should throw an error on the "watchFiles" option with '{"options":{"invalid":true}}' value 1`] = `
"ValidationError: Invalid options object. Dev Server has been initialized using an options object that does not match the API schema.
- options.watchFiles.options has an unknown property 'invalid'. These properties are valid:
object { persistent?, ignored?, ignoreInitial?, followSymlinks?, cwd?, disableGlobbing?, usePolling?, interval?, binaryInterval?, alwaysStat?, depth?, awaitWriteFinish?, ignorePermissionErrors?, atomic? }
-> Configure advanced options for watching. See the chokidar documentation for the possible options.
-> Read more at https://github.com/paulmillr/chokidar#api"
`;

exports[`options validate should throw an error on the "watchFiles" option with '{"options":false}' value 1`] = `
"ValidationError: Invalid options object. Dev Server has been initialized using an options object that does not match the API schema.
- options.watchFiles.options should be an object:
object { … }
object { persistent?, ignored?, ignoreInitial?, followSymlinks?, cwd?, disableGlobbing?, usePolling?, interval?, binaryInterval?, alwaysStat?, depth?, awaitWriteFinish?, ignorePermissionErrors?, atomic? }
-> Configure advanced options for watching. See the chokidar documentation for the possible options.
-> Read more at https://github.com/paulmillr/chokidar#api"
`;
Expand Down
17 changes: 17 additions & 0 deletions test/validate-options.test.js
View file
Open in desktop
Expand Up @@ -485,6 +485,15 @@ const tests = {
serveIndex: {},
watch: {},
},
{
directory: "path",
staticOptions: {},
publicPath: ["/public1/", "/public2/"],
watch: {
ignored: "*.txt",
usePolling: true,
},
},
[
"path1",
{
Expand Down Expand Up @@ -512,6 +521,9 @@ const tests = {
{
watch: 10,
},
{
watch: { invalid: true },
},
],
},
setupMiddlewares: {
Expand Down Expand Up @@ -571,6 +583,11 @@ const tests = {
{
options: false,
},
{
options: {
invalid: true,
},
},
],
},
};
Expand Down
70 changes: 59 additions & 11 deletions types/lib/Server.d.ts
View file
Open in desktop
Expand Up @@ -2724,13 +2724,11 @@ declare class Server {
cli: {
negatedDescription: string;
};
description?: undefined;
link?: undefined;
$ref?: undefined;
}
| {
type: string;
description: string;
link: string;
$ref: string;
type?: undefined;
cli?: undefined /** @typedef {import("express").Request} Request */;
}
)[];
Expand Down Expand Up @@ -2782,16 +2780,13 @@ declare class Server {
| {
type: string;
minLength: number;
/** @type {ServerConfiguration} */ items?: undefined;
items?: undefined;
}
)[];
description: string;
};
options: {
type: string;
description: string;
link: string;
additionalProperties: boolean;
$ref: string;
};
};
additionalProperties: boolean;
Expand All @@ -2800,11 +2795,64 @@ declare class Server {
type: string;
minLength: number;
};
WatchOptionsChokidar: {
type: string;
additionalProperties: boolean;
description: string;
link: string;
properties: {
persistent: {
type: string;
};
ignored: {
type: string;
};
ignoreInitial: {
type: string;
};
followSymlinks: {
type: string /** @type {ServerOptions} */;
};
cwd: {
type: string;
};
disableGlobbing: {
type: string;
};
usePolling: {
type: string;
};
interval: {
type: string;
};
binaryInterval: {
type: string;
};
alwaysStat: {
type: string;
};
depth: {
type: string /** @type {ServerOptions} */;
};
awaitWriteFinish: {
type: string;
additionalProperties: boolean;
};
ignorePermissionErrors: {
type: string;
};
atomic: {
anyOf: {
type: string;
}[];
};
};
};
WebSocketServer: {
anyOf: {
$ref: string;
}[];
/** @type {ServerOptions} */ description: string;
description: string;
link: string;
};
WebSocketServerType: {
Expand Down