feat(client): close vite-error-overlay with Escape key

[chaejunlee]

Description

fixes #13748

Pressing esc key closes the error overlay.

I made sure to remove the EventListener after the overlay is closed.

Additional context

I added an 'keydown' EventListener to document.

Please let me know if it is inappropriate.

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

Before submitting the PR, please make sure you do the following

• Read the Contributing Guidelines.
• Read the Pull Request Guidelines and follow the PR Title Convention.
• Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
• Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
• Ideally, include relevant tests that fail without this PR but pass with it.

[stackblitz]

Run & review this pull request in StackBlitz Codeflow.

[Henry-Hong]

Looking good.

[chaejunlee]

While adding test, I found out that pressing Escape key doesn't close the overlay.

It was because of this object not directing to the correct object when passed as an addEventListener callback.

Therefore, I localized close function in order to remove the 'keydown' EventListener when the overlay is closed

[chaejunlee]

@bluwy

I have put the close() back as the member function.

How I solved it was to assign (e => {...}).bind(this) to this.closeOnEsc().

Due to the fact that .bind(this) creates new function, document.addEventListener('keydown', this.closeOnEsc.bind(this)) makes it impossible to remove the keydown listener.

I've checked that setting this.closeOnEsc = (e) => {} fails on the test that I added.

[bluwy]

Looks great!

[chaejunlee]

Thanks a lot for the approval! @bluwy

I don't think we can remove this, or it'll break:

vite/packages/vite/src/client/client.ts

Lines 297 to 301 in ec9d2e7

	function clearErrorOverlay() {
	document
	.querySelectorAll(overlayId)
	.forEach((n) => (n as ErrorOverlay).close())
	}

Regarding the code that you provided, I think there are no tests around handling the "message" events of WebSocket, especially 'update' and 'error'.

If that's the case, do you mind if I open a new issue and add some tests, after this PR?

[bluwy]

For sure feel free to followup on that. We currently cover that part of the code in the hmr playground, so any additions to make it more robust is definitely appreciated.

[patak-dev]

Nice! @bluwy do you think we can merge this in a patch? It is a new feature so the safest may be to add it to Vite 5 milestone.

[bluwy]

Yeah I think we can merge this as patch too. Was hoping to get another eye before merging.