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 all 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
79 changes: 64 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 All @@ -27,6 +35,30 @@
* @property {string} [previousHash]
*/

/**
* @param {boolean | { warnings?: boolean | string; errors?: boolean | string; runtimeErrors?: boolean | string; }} overlayOptions
*/
const decodeOverlayOptions = (overlayOptions) => {
if (typeof overlayOptions === "object") {
["warnings", "errors", "runtimeErrors"].forEach((property) => {
if (typeof overlayOptions[property] === "string") {
const overlayFilterFunctionString = decodeURIComponent(

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

View check run for this annotation

Codecov / codecov/patch

client-src/index.js#L45 

Added line #L45 was not covered by tests
overlayOptions[property]
);

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

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

View check run for this annotation

Codecov / codecov/patch

client-src/index.js#L50 

Added line #L50 was not covered by tests
"message",
`var callback = ${overlayFilterFunctionString}
return callback(message)`
);
Comment on lines +50 to +54
Copy link

Choose a reason for hiding this comment

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

this new Function() breaks devserver for chrome extensions with the following error

Uncaught EvalError: Refused to evaluate a string as JavaScript because 'unsafe-eval' is not an allowed source of script in the following Content Security Policy directive: "script-src 'self'".

it could be fixed by adding 'unsafe-eval' in the manifest but that isn't really an option for us so we're going to roll back for now

Copy link
Member

Choose a reason for hiding this comment

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

@wentokay We can optionally inject this, can you open an issue?


overlayOptions[property] = overlayFilterFunction;

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

View check run for this annotation

Codecov / codecov/patch

client-src/index.js#L56 

Added line #L56 was not covered by tests
}
});
}
};

/**
* @type {Status}
*/
Expand Down Expand Up @@ -83,6 +115,8 @@
runtimeErrors: true,
...options.overlay,
};

decodeOverlayOptions(options.overlay);
}
enabledFeatures.Overlay = true;
}
Expand Down Expand Up @@ -173,6 +207,7 @@
}

options.overlay = value;
decodeOverlayOptions(options.overlay);
},
/**
* @param {number} value
Expand Down Expand Up @@ -266,17 +301,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 312 in client-src/index.js

View check run for this annotation

Codecov / codecov/patch

client-src/index.js#L312 

Added line #L312 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 +345,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 356 in client-src/index.js

View check run for this annotation

Codecov / codecov/patch

client-src/index.js#L356 

Added line #L356 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