Skip to content

feat: allow filter overlay message with function #4813

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.

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

Merged
merged 6 commits into from
May 6, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 54 additions & 15 deletions client-src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,20 @@ import sendMessage from "./utils/sendMessage.js";
import reloadApp from "./utils/reloadApp.js";
import createSocketURL from "./utils/createSocketURL.js";

/**
* @typedef {Object} OverlayOptions
* @property {boolean | (error: Error) => boolean} [warnings]
* @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 @@ if (parsedResourceQuery.overlay) {
runtimeErrors: true,
...options.overlay,
};

["errors", "warnings", "runtimeErrors"].forEach((property) => {
if (typeof options.overlay[property] === "string") {
const overlayFilterFunctionString = decodeURIComponent(
options.overlay[property]
);

// eslint-disable-next-line no-new-func
const overlayFilterFunction = new Function(
"message",
`var callback = ${overlayFilterFunctionString}
return callback(message)`
);

options.overlay[property] = overlayFilterFunction;
}
});
}
enabledFeatures.Overlay = true;
}
Expand Down Expand Up @@ -266,17 +291,24 @@ const onSocketMessage = {
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)
: 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:

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 @@ const onSocketMessage = {
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)
: 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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
Expand Up @@ -154,10 +154,14 @@ const schema = require("./options.json");
* @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 @@ class Server {
}

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

@malcolm-kee malcolm-kee Apr 11, 2023

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
Loading