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

feat: allow filter overlay message with function #4813

Merged
merged 6 commits into from May 6, 2023
Merged

feat: allow filter overlay message with function #4813

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4658fde
feat: allow filter overlay message with function
malcolm-kee May 5, 2023
9e63422
fix: update snapshot
malcolm-kee May 5, 2023
9c6e408
test: add test for overlay runtimeErrors
malcolm-kee May 5, 2023
6c017eb
test: test overlay errors and warnings filter function
malcolm-kee May 5, 2023
2132c30
fix: encode overlay options correctly
malcolm-kee May 6, 2023
f7386f6
fix: update types
malcolm-kee May 6, 2023
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
69 changes: 54 additions & 15 deletions client-src/index.js
View file
Open in desktop
Expand Up @@ -10,12 +10,20 @@
import reloadApp from "./utils/reloadApp.js";
import createSocketURL from "./utils/createSocketURL.js";

/**
* @typedef {Object} OverlayOptions
* @property {boolean | (error: Error) => boolean} [warnings]
malcolm-kee marked this conversation as resolved.
Show resolved Hide resolved
* @property {boolean | (error: Error) => boolean} [errors]
* @property {boolean | (error: Error) => boolean} [runtimeErrors]
* @property {string} [trustedTypesPolicyName]
*/

/**
* @typedef {Object} Options
* @property {boolean} hot
* @property {boolean} liveReload
* @property {boolean} progress
* @property {boolean | { warnings?: boolean, errors?: boolean, runtimeErrors?: boolean, trustedTypesPolicyName?: string }} overlay
* @property {boolean | OverlayOptions} overlay
* @property {string} [logging]
* @property {number} [reconnect]
*/
Expand Down Expand Up @@ -83,6 +91,23 @@
runtimeErrors: true,
...options.overlay,
};

["errors", "warnings", "runtimeErrors"].forEach((property) => {
if (typeof options.overlay[property] === "string") {
malcolm-kee marked this conversation as resolved.
Show resolved Hide resolved
const overlayFilterFunctionString = decodeURIComponent(

Check warning on line 97 in client-src/index.js

View check run for this annotation

Codecov / codecov/patch

client-src/index.js#L97 

Added line #L97 was not covered by tests
options.overlay[property]
);

// eslint-disable-next-line no-new-func
const overlayFilterFunction = new Function(

Check warning on line 102 in client-src/index.js

View check run for this annotation

Codecov / codecov/patch

client-src/index.js#L102 

Added line #L102 was not covered by tests
"message",
`var callback = ${overlayFilterFunctionString}
return callback(message)`
);

options.overlay[property] = overlayFilterFunction;

Check warning on line 108 in client-src/index.js

View check run for this annotation

Codecov / codecov/patch

client-src/index.js#L108 

Added line #L108 was not covered by tests
}
});
}
enabledFeatures.Overlay = true;
}
Expand Down Expand Up @@ -266,17 +291,24 @@
log.warn(printableWarnings[i]);
}

const needShowOverlayForWarnings =
const overlayWarningsSetting =
typeof options.overlay === "boolean"
? options.overlay
: options.overlay && options.overlay.warnings;

if (needShowOverlayForWarnings) {
overlay.send({
type: "BUILD_ERROR",
level: "warning",
messages: warnings,
});
if (overlayWarningsSetting) {
const warningsToDisplay =
typeof overlayWarningsSetting === "function"
? warnings.filter(overlayWarningsSetting)

Check warning on line 302 in client-src/index.js

View check run for this annotation

Codecov / codecov/patch

client-src/index.js#L302 

Added line #L302 was not covered by tests
: warnings;

if (warningsToDisplay.length) {
overlay.send({
type: "BUILD_ERROR",
level: "warning",
messages: warnings,
Comment on lines +317 to +319
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be BUILD_WARNING?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the event type purpose is to determine how to transition the state:

webpack-dev-server/client-src/overlay/state-machine.js

Lines 32 to 38 in f7386f6

BUILD_ERROR: {
target: "displayBuildError",
actions: ["setMessages", "showOverlay"],
},
RUNTIME_ERROR: {
target: "displayRuntimeError",
actions: ["setMessages", "showOverlay"],

I don't mind refactor to add event type BUILD_WARNING and remove level property, although it does introduce some duplication in the state machine.

Copy link
Member

Choose a reason for hiding this comment

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

I think it's fine 👍🏻

});
}
}

if (params && params.preventReloading) {
Expand All @@ -303,17 +335,24 @@
log.error(printableErrors[i]);
}

const needShowOverlayForErrors =
const overlayErrorsSettings =
typeof options.overlay === "boolean"
? options.overlay
: options.overlay && options.overlay.errors;

if (needShowOverlayForErrors) {
overlay.send({
type: "BUILD_ERROR",
level: "error",
messages: errors,
});
if (overlayErrorsSettings) {
const errorsToDisplay =
typeof overlayErrorsSettings === "function"
? errors.filter(overlayErrorsSettings)

Check warning on line 346 in client-src/index.js

View check run for this annotation

Codecov / codecov/patch

client-src/index.js#L346 

Added line #L346 was not covered by tests
: errors;

if (errorsToDisplay.length) {
overlay.send({
type: "BUILD_ERROR",
level: "error",
messages: errors,
});
}
}
},
/**
Expand Down
44 changes: 30 additions & 14 deletions client-src/overlay.js
View file
Open in desktop
Expand Up @@ -78,7 +78,7 @@ function formatProblem(type, item) {
/**
* @typedef {Object} CreateOverlayOptions
* @property {string | null} trustedTypesPolicyName
* @property {boolean} [catchRuntimeError]
* @property {boolean | (error: Error) => void} [catchRuntimeError]
*/

/**
Expand All @@ -90,6 +90,8 @@ const createOverlay = (options) => {
let iframeContainerElement;
/** @type {HTMLDivElement | null | undefined} */
let containerElement;
/** @type {HTMLDivElement | null | undefined} */
let headerElement;
/** @type {Array<(element: HTMLDivElement) => void>} */
let onLoadQueue = [];
/** @type {TrustedTypePolicy | undefined} */
Expand Down Expand Up @@ -124,6 +126,7 @@ const createOverlay = (options) => {
iframeContainerElement.id = "webpack-dev-server-client-overlay";
iframeContainerElement.src = "about:blank";
applyStyle(iframeContainerElement, iframeStyle);

iframeContainerElement.onload = () => {
const contentElement =
/** @type {Document} */
Expand All @@ -141,7 +144,7 @@ const createOverlay = (options) => {
contentElement.id = "webpack-dev-server-client-overlay-div";
applyStyle(contentElement, containerStyle);

const headerElement = document.createElement("div");
headerElement = document.createElement("div");

headerElement.innerText = "Compiled with problems:";
applyStyle(headerElement, headerStyle);
Expand Down Expand Up @@ -219,9 +222,15 @@ const createOverlay = (options) => {
* @param {string} type
* @param {Array<string | { moduleIdentifier?: string, moduleName?: string, loc?: string, message?: string }>} messages
* @param {string | null} trustedTypesPolicyName
* @param {'build' | 'runtime'} messageSource
*/
function show(type, messages, trustedTypesPolicyName) {
function show(type, messages, trustedTypesPolicyName, messageSource) {
ensureOverlayExists(() => {
headerElement.innerText =
messageSource === "runtime"
? "Uncaught runtime errors:"
: "Compiled with problems:";

messages.forEach((message) => {
const entryElement = document.createElement("div");
const msgStyle =
Expand Down Expand Up @@ -267,8 +276,8 @@ const createOverlay = (options) => {
}

const overlayService = createOverlayMachine({
showOverlay: ({ level = "error", messages }) =>
show(level, messages, options.trustedTypesPolicyName),
showOverlay: ({ level = "error", messages, messageSource }) =>
show(level, messages, options.trustedTypesPolicyName, messageSource),
hideOverlay: hide,
});

Expand All @@ -284,15 +293,22 @@ const createOverlay = (options) => {
const errorObject =
error instanceof Error ? error : new Error(error || message);

overlayService.send({
type: "RUNTIME_ERROR",
messages: [
{
message: errorObject.message,
stack: parseErrorToStacks(errorObject),
},
],
});
const shouldDisplay =
typeof options.catchRuntimeError === "function"
? options.catchRuntimeError(errorObject)
: true;

if (shouldDisplay) {
overlayService.send({
type: "RUNTIME_ERROR",
messages: [
{
message: errorObject.message,
stack: parseErrorToStacks(errorObject),
},
],
});
}
});
}

Expand Down
5 changes: 5 additions & 0 deletions client-src/overlay/state-machine.js
View file
Open in desktop
Expand Up @@ -4,6 +4,7 @@ import createMachine from "./fsm.js";
* @typedef {Object} ShowOverlayData
* @property {'warning' | 'error'} level
* @property {Array<string | { moduleIdentifier?: string, moduleName?: string, loc?: string, message?: string }>} messages
* @property {'build' | 'runtime'} messageSource
*/

/**
Expand All @@ -23,6 +24,7 @@ const createOverlayMachine = (options) => {
context: {
level: "error",
messages: [],
messageSource: "build",
},
states: {
hidden: {
Expand Down Expand Up @@ -73,18 +75,21 @@ const createOverlayMachine = (options) => {
return {
messages: [],
level: "error",
messageSource: "build",
};
},
appendMessages: (context, event) => {
return {
messages: context.messages.concat(event.messages),
level: event.level || context.level,
messageSource: event.type === "RUNTIME_ERROR" ? "runtime" : "build",
};
},
setMessages: (context, event) => {
return {
messages: event.messages,
level: event.level || context.level,
messageSource: event.type === "RUNTIME_ERROR" ? "runtime" : "build",
};
},
hideOverlay,
Expand Down
10 changes: 9 additions & 1 deletion examples/client/overlay/app.js
View file
Open in desktop
Expand Up @@ -5,7 +5,15 @@ const createErrorBtn = require("./error-button");

const target = document.querySelector("#target");

target.insertAdjacentElement("afterend", createErrorBtn());
target.insertAdjacentElement(
"afterend",
createErrorBtn("Click to throw error", "Error message thrown from JS")
);

target.insertAdjacentElement(
"afterend",
createErrorBtn("Click to throw ignored error", "something something")
);

// eslint-disable-next-line import/no-unresolved, import/extensions
const invalid = require("./invalid.js");
Expand Down
22 changes: 14 additions & 8 deletions examples/client/overlay/error-button.js
View file
Open in desktop
@@ -1,18 +1,24 @@
"use strict";

function unsafeOperation() {
throw new Error("Error message thrown from JS");
}
/**
*
* @param {string} label
* @param {string} errorMessage
* @returns HTMLButtonElement
*/
module.exports = function createErrorButton(label, errorMessage) {
function unsafeOperation() {
throw new Error(errorMessage);
}

function handleButtonClick() {
unsafeOperation();
}
function handleButtonClick() {
unsafeOperation();
}

module.exports = function createErrorButton() {
const errorBtn = document.createElement("button");

errorBtn.addEventListener("click", handleButtonClick);
errorBtn.innerHTML = "Click to throw error";
errorBtn.innerHTML = label;

return errorBtn;
};
21 changes: 20 additions & 1 deletion examples/client/overlay/webpack.config.js
View file
Open in desktop
Expand Up @@ -10,7 +10,26 @@ module.exports = setup({
entry: "./app.js",
devServer: {
client: {
overlay: true,
overlay: {
warnings: false,
runtimeErrors: (msg) => {
if (msg) {
let msgString;

if (msg instanceof Error) {
msgString = msg.message;
} else if (typeof msg === "string") {
msgString = msg;
}

if (msgString) {
return !/something/i.test(msgString);
}
}

return true;
},
},
},
},
// uncomment to test for IE
Expand Down
30 changes: 25 additions & 5 deletions lib/Server.js
View file
Open in desktop
Expand Up @@ -154,10 +154,14 @@
* @property {string} [username]
*/

/**
* @typedef {boolean | ((error: Error) => void)} OverlayMessageOptions
*/

/**
* @typedef {Object} ClientConfiguration
* @property {"log" | "info" | "warn" | "error" | "none" | "verbose"} [logging]
* @property {boolean | { warnings?: boolean, errors?: boolean, runtimeErrors?: boolean }} [overlay]
* @property {boolean | { warnings?: OverlayMessageOptions, errors?: OverlayMessageOptions, runtimeErrors?: OverlayMessageOptions }} [overlay]
* @property {boolean} [progress]
* @property {boolean | number} [reconnect]
* @property {"ws" | "sockjs" | string} [webSocketTransport]
Expand Down Expand Up @@ -654,12 +658,28 @@
}

if (typeof client.overlay !== "undefined") {
searchParams.set(
"overlay",
/**
*
* @param {OverlayMessageOptions} [setting]
* @returns
*/
const encodeOverlaySettings = (setting) =>
typeof setting === "function"
? encodeURIComponent(setting.toString())

Check warning on line 668 in lib/Server.js

View check run for this annotation

Codecov / codecov/patch

lib/Server.js#L668 

Added line #L668 was not covered by tests
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Usage of encodeURIComponent here is intentional, because without it URLSearchParams will encode white space into + character, which could not be decoded correctly in parseURL in client.

Alternative is to use URLSearchParams in client side, but I think the drawback of that is cannot support old browser.

: setting;

const overlayString =
typeof client.overlay === "boolean"
? String(client.overlay)
: JSON.stringify(client.overlay)
);
: JSON.stringify({
errors: encodeOverlaySettings(client.overlay.errors),
warnings: encodeOverlaySettings(client.overlay.warnings),
runtimeErrors: encodeOverlaySettings(
client.overlay.runtimeErrors
),
});

searchParams.set("overlay", overlayString);
}

if (typeof client.reconnect !== "undefined") {
Expand Down