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.

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

feat: open editor when clicking error on overlay #4587

Merged
merged 2 commits into from
Oct 12, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
12 changes: 11 additions & 1 deletion client-src/overlay.js
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ function formatProblem(type, item) {
// Compilation with errors (e.g. syntax error or missing modules).
/**
* @param {string} type
* @param {Array<string | { file?: string, moduleName?: string, loc?: string, message?: string }>} messages
* @param {Array<string | { moduleIdentifier?: string, moduleName?: string, loc?: string, message?: string }>} messages
* @param {string | null} trustedTypesPolicyName
*/
function show(type, messages, trustedTypesPolicyName) {
Expand All @@ -203,6 +203,16 @@ function show(type, messages, trustedTypesPolicyName) {
typeElement.innerText = header;
applyStyle(typeElement, msgTypeStyle);

if (message.moduleIdentifier) {
applyStyle(typeElement, { cursor: "pointer" });
typeElement.dataset.canOpen = true;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is hook for e2e test. If there is other preferred convention, let me know!

typeElement.addEventListener("click", () => {
fetch(
`/webpack-dev-server/open-editor?fileName=${message.moduleIdentifier}`
);
});
}

// Make it look similar to our terminal.
const text = ansiHTML(encode(body));
const messageTextNode = document.createElement("div");
Expand Down
4 changes: 4 additions & 0 deletions examples/client/overlay/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,9 @@

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

// eslint-disable-next-line import/no-unresolved, import/extensions
const invalid = require("./invalid.js");

console.log(invalid);
target.classList.add("pass");
target.innerHTML = "Success!";
2 changes: 1 addition & 1 deletion examples/client/overlay/webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const { setup } = require("../../util");
module.exports = setup({
context: __dirname,
// create error for overlay
entry: "./invalid.js",
entry: "./app.js",
devServer: {
client: {
overlay: true,
Expand Down
13 changes: 13 additions & 0 deletions lib/Server.js
Original file line number Diff line number Diff line change
Expand Up @@ -2033,6 +2033,19 @@ class Server {
}
);

/** @type {import("express").Application} */
(app).get("/webpack-dev-server/open-editor", (req, res) => {
const fileName = req.query.fileName;

if (typeof fileName === "string") {
// @ts-ignore
const launchEditor = require("launch-editor");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

require lazily to make mock possible.

launchEditor(fileName);
}

res.end();
});

/** @type {import("express").Application} */
(app).get(
"/webpack-dev-server",
Expand Down
38 changes: 34 additions & 4 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
"html-entities": "^2.3.2",
"http-proxy-middleware": "^2.0.3",
"ipaddr.js": "^2.0.1",
"launch-editor": "^2.6.0",
"open": "^8.0.9",
"p-retry": "^4.5.0",
"rimraf": "^3.0.2",
Expand Down Expand Up @@ -126,6 +127,7 @@
"tcp-port-used": "^1.0.2",
"typescript": "^4.7.2",
"url-loader": "^4.1.1",
"wait-for-expect": "^3.0.2",
"webpack": "^5.71.0",
"webpack-cli": "^4.7.2",
"webpack-merge": "^5.8.0"
Expand Down
8 changes: 8 additions & 0 deletions test/e2e/__snapshots__/overlay.test.js.snap.webpack5
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,13 @@ exports[`overlay should not show initially, then show on an error and allow to c
\\"
>
<div
data-can-open=\\"true\\"
style=\\"
color: rgb(232, 59, 70);
font-size: 1.2em;
margin-bottom: 1rem;
font-family: sans-serif;
cursor: pointer;
\\"
>
ERROR in ./foo.js 1:1
Expand Down Expand Up @@ -212,11 +214,13 @@ exports[`overlay should not show initially, then show on an error, then hide on
\\"
>
<div
data-can-open=\\"true\\"
style=\\"
color: rgb(232, 59, 70);
font-size: 1.2em;
margin-bottom: 1rem;
font-family: sans-serif;
cursor: pointer;
\\"
>
ERROR in ./foo.js 1:1
Expand Down Expand Up @@ -334,11 +338,13 @@ exports[`overlay should not show initially, then show on an error, then show oth
\\"
>
<div
data-can-open=\\"true\\"
style=\\"
color: rgb(232, 59, 70);
font-size: 1.2em;
margin-bottom: 1rem;
font-family: sans-serif;
cursor: pointer;
\\"
>
ERROR in ./foo.js 1:1
Expand Down Expand Up @@ -419,11 +425,13 @@ exports[`overlay should not show initially, then show on an error, then show oth
\\"
>
<div
data-can-open=\\"true\\"
style=\\"
color: rgb(232, 59, 70);
font-size: 1.2em;
margin-bottom: 1rem;
font-family: sans-serif;
cursor: pointer;
\\"
>
ERROR in ./foo.js 1:1
Expand Down
50 changes: 50 additions & 0 deletions test/e2e/overlay.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const path = require("path");
const fs = require("graceful-fs");
const prettier = require("prettier");
const webpack = require("webpack");
const waitForExpect = require("wait-for-expect");
const Server = require("../../lib/Server");
const config = require("../fixtures/overlay-config/webpack.config");
const trustedTypesConfig = require("../fixtures/overlay-config/trusted-types.webpack.config");
Expand Down Expand Up @@ -478,6 +479,55 @@ describe("overlay", () => {
await server.stop();
});

(isWebpack5 ? it : it.skip)(
"should open editor when error with file info is clicked",
async () => {
const mockLaunchEditorCb = jest.fn();
jest.mock("launch-editor", () => mockLaunchEditorCb);

const compiler = webpack(config);
const devServerOptions = {
port,
};
const server = new Server(devServerOptions, compiler);

await server.start();

const { page, browser } = await runBrowser();

await page.goto(`http://localhost:${port}/`, {
waitUntil: "networkidle0",
});

const pathToFile = path.resolve(
__dirname,
"../fixtures/overlay-config/foo.js"
);
const originalCode = fs.readFileSync(pathToFile);

fs.writeFileSync(pathToFile, "`;");

await page.waitForSelector("#webpack-dev-server-client-overlay");

const frame = page
.frames()
.find((item) => item.name() === "webpack-dev-server-client-overlay");

const errorHandle = await frame.$("[data-can-open]");

await errorHandle.click();

await waitForExpect(() => {
expect(mockLaunchEditorCb).toHaveBeenCalledTimes(1);
});

fs.writeFileSync(pathToFile, originalCode);

await browser.close();
await server.stop();
}
);

it('should not show a warning when "client.overlay" is "false"', async () => {
const compiler = webpack(config);

Expand Down
16 changes: 16 additions & 0 deletions test/helpers/run-browser.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,16 @@
const puppeteer = require("puppeteer");
const { puppeteerArgs } = require("./puppeteer-constants");

/**
* @typedef {Object} RunBrowserResult
* @property {import('puppeteer').Page} page
* @property {import('puppeteer').Browser} browser
*/

/**
* @param {Parameters<import('puppeteer').Page['emulate']>[0]} config
* @returns {Promise<RunBrowserResult>}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Additional typing to improve intellisense while writing tests.

*/
function runBrowser(config) {
const options = {
viewport: {
Expand All @@ -14,7 +24,13 @@ function runBrowser(config) {
};

return new Promise((resolve, reject) => {
/**
* @type {import('puppeteer').Page}
*/
let page;
/**
* @type {import('puppeteer').Browser}
*/
let browser;

puppeteer
Expand Down