feat(qwik): customize error overlay #2818

zanettin · 2023-02-03T21:54:42Z

What is it?

Feature / enhancement
Bug
Docs / tests

Description

Qwikify the vite dev server error overlay. Fixes #2710.

Current Version

PR state after review session

Terminal output (stays unaffected)

Info

Used Icons are from https://www.svgrepo.com

Todos

Get feedback regarding design
Wait till this PR refactor: customize ErrorOverlay (part 2) vitejs/vite#11830 gets merged
Wait till qwik uses version 4.2.0 of vite which includes the changes from above

Follow-ups

Remove vite code after feat: customize error middleware vitejs/vite#11970 got merged

Checklist:

My code follows the developer guidelines of this project
I have performed a self-review of my own code
I have made corresponding changes to the documentation
Added new tests to cover the fix / functionality

stackblitz · 2023-02-03T21:54:45Z

Run & review this pull request in StackBlitz Codeflow.

zanettin · 2023-02-03T21:56:52Z

packages/qwik/package.json

@@ -115,7 +115,8 @@
  ],
  "devDependencies": {
    "@builder.io/qwik-dom": "2.1.19",
-    "kleur": "4.1.5"
+    "kleur": "4.1.5",
+    "strip-ansi": "7.0.1"


used on vite to strip the the frame output on the error overlay. since we have the code included atm, we need the lib as well

zanettin · 2023-02-03T22:00:27Z

packages/qwik-city/buildtime/vite/dev-server.ts

@@ -194,6 +194,9 @@ export function ssrDevMiddleware(ctx: BuildContext, server: ViteDevServer) {

      next();
    } catch (e) {
+      if (res.writableEnded) {


we've extended the qwik dev server middleware to handle errors as well instead of forwarding it to the vite error middleware. this because we are otherwise unable to ship the needed CSS to modify the overlay on SSR errors. The new error handler terminates the response with an .end() call. After that point it does no longer make sense to follow the next(e) path at all because an header already sent error would occur.

zanettin · 2023-02-03T22:02:59Z

packages/qwik/src/optimizer/src/plugins/vite-error.ts

+import { yellow, magenta, cyan } from 'kleur/colors';
+import strip from 'strip-ansi';
+
+/**


i think about creating another PR on the vite repo to have an option to pass in some custom markdown into the vite error handler so we can remove the whole logic again on our side.

zanettin · 2023-02-03T22:03:49Z

packages/qwik/src/optimizer/src/plugins/vite-error.ts

+}
+
+vite-error-overlay::part(tip):before {
+  content: "Not sure how to solve this? Visit https://qwik.builder.io or connect with the community on Discord.";


just an idea. please let me know what u think about it...

yeah if this works! i think it's ok!

it does but w/o a link 😄 and can't style the url

manucorporat · 2023-02-04T09:18:48Z

THIS IS AMAZING!! love it, this will deserve a good tweet when we merge it

shairez · 2023-02-04T13:52:43Z

great job @zanettin !

Looks much more readable

BTW, for errors, I'd love to see the color "red" because it makes it easier to see that it's an error, immediately

Plus, not sure if it's possible, but syntax highlighting would also be great, but that's not a much

manucorporat · 2023-02-04T15:26:59Z

I dont think code highlighting is an easy thing to add, i would not block this PR for that, we never had it before, but i would say, i would like to see the code with a more vibrant color, the yellow for vite was cooler.

manucorporat · 2023-02-04T15:33:47Z

My only take would be to add a little bit of color, the red @shairez suggested would be nice and some other

zanettin · 2023-02-04T16:13:36Z

thanks a lot @manucorporat and @shairez for your feedbacks 🙏

the concern @shairez mentioned was about the terminal output which would stay unaffected by now.
i'll try to add some more colors to the overlay and post an update asap 🙏

regarding the formatting/highlighting: yes we are limited as long as we'd like to keep the vite error overlay web component since the error is rendered within a <pre> tag. if we would like to do everything on our side (other PR imo as well) we could do that which would give us complete flexibility 👍

zanettin · 2023-02-04T20:30:11Z

@manucorporat @shairez here are some other variations 👨‍🎨 😄
pls let me know in which direction i should go or if one of them match your expectations 🙏 i've also updated the description of the PR and added a screenshot of the terminal output.

B1

B2

B3

C1

shairez · 2023-02-04T20:46:06Z

thanks @zanettin !

I prefer B2

zanettin · 2023-02-04T20:58:00Z

thanks @zanettin !

I prefer B2

thanks @shairez 🙏 maybe i should play around a bit with the aggressivity of the red color? give it a bit of opacity or so 👍
imo a bit less "qwik"-style than the other options but it is an error overlay after all 😄

shairez · 2023-02-04T21:07:34Z

I think it's OK not to have the error message with the branded blue color, because it's an error after all :)

Maybe the icons could be with that blue or something, but it's not a biggie IMO

zanettin · 2023-02-04T22:20:20Z

totally with u @shairez 🙏 let me try some variations tomorrow evening 👍

zanettin · 2023-02-05T22:13:12Z

B4

tiny adjusted B2 version (reduced red, removed blue completely from the screen)

nnelgxorz · 2023-02-06T04:40:04Z

I think it looks great.

I personally prefer not having the error message header as red and reserving the red color for highlighting the actual error in the code, like this. Giant red text just makes me feel like I'm being yelled at.

I would bet that it stresses out beginners as well. Just my two cents. 👍

shairez · 2023-02-06T09:35:37Z

but you ARE being yelled at @nnelgxorz !! 😅😂

adamdbradley · 2023-02-06T16:32:02Z

Love it, and like the idea of less red and more qwik-ish colors. But think we ship it and see how it feels and iterate.

zanettin · 2023-02-06T20:52:53Z

thanks for all the feedbacks 🙏 set the current state of this PR to the B1 version. hope that is a good starting point. if you have other wishes/requirements, pls let me know 🙏
otherwise i'll wait till the PR on the vite repo got merged (was approved twice now 🎉 ) and we've upgraded to latest vite version for final testings.

manucorporat · 2023-02-08T14:34:54Z

B1 is cool!

zanettin added 3 commits February 3, 2023 21:41

🧹

a5d4e9d

✨

aac4d90

🎨

31593b9

zanettin commented Feb 3, 2023

View reviewed changes

qwik-ish

81732c9

zanettin mentioned this pull request Feb 7, 2023

feat: customize error middleware vitejs/vite#11970

Closed

9 tasks

Merge branch 'main' into feature/error-overlay

fd49743

zanettin added the COMP: DX Developer Experience related issue label Feb 15, 2023

zanettin added 4 commits March 16, 2023 21:32

Merge branch 'main' into feature/error-overlay

20f7fd6

Merge branch 'main' into feature/error-overlay

e8e7afa

🚁

9f1d5b6

🏈

0058798

zanettin marked this pull request as ready for review March 17, 2023 22:00

manucorporat added 3 commits March 18, 2023 14:16

Merge branch 'main' into feature/error-overlay

8a00383

Merge branch 'main' into feature/error-overlay

1c1acae

🐝

9acc9ef

manucorporat enabled auto-merge (squash) March 18, 2023 13:58

manucorporat merged commit bf9330f into QwikDev:main Mar 18, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(qwik): customize error overlay #2818

feat(qwik): customize error overlay #2818

zanettin commented Feb 3, 2023 •

edited

stackblitz bot commented Feb 3, 2023

zanettin Feb 3, 2023

zanettin Feb 3, 2023 •

edited

zanettin Feb 3, 2023

zanettin Feb 3, 2023

manucorporat Feb 4, 2023

zanettin Feb 4, 2023

manucorporat commented Feb 4, 2023

shairez commented Feb 4, 2023 •

edited

manucorporat commented Feb 4, 2023

manucorporat commented Feb 4, 2023

zanettin commented Feb 4, 2023

zanettin commented Feb 4, 2023

shairez commented Feb 4, 2023

zanettin commented Feb 4, 2023

shairez commented Feb 4, 2023

zanettin commented Feb 4, 2023

zanettin commented Feb 5, 2023

nnelgxorz commented Feb 6, 2023

shairez commented Feb 6, 2023

adamdbradley commented Feb 6, 2023

zanettin commented Feb 6, 2023

manucorporat commented Feb 8, 2023

feat(qwik): customize error overlay #2818

feat(qwik): customize error overlay #2818

Conversation

zanettin commented Feb 3, 2023 • edited

What is it?

Description

Current Version

PR state after review session

Terminal output (stays unaffected)

Info

Todos

Follow-ups

Checklist:

stackblitz bot commented Feb 3, 2023

zanettin Feb 3, 2023

Choose a reason for hiding this comment

zanettin Feb 3, 2023 • edited

Choose a reason for hiding this comment

zanettin Feb 3, 2023

Choose a reason for hiding this comment

zanettin Feb 3, 2023

Choose a reason for hiding this comment

manucorporat Feb 4, 2023

Choose a reason for hiding this comment

zanettin Feb 4, 2023

Choose a reason for hiding this comment

manucorporat commented Feb 4, 2023

shairez commented Feb 4, 2023 • edited

manucorporat commented Feb 4, 2023

manucorporat commented Feb 4, 2023

zanettin commented Feb 4, 2023

zanettin commented Feb 4, 2023

B1

B2

B3

C1

shairez commented Feb 4, 2023

zanettin commented Feb 4, 2023

shairez commented Feb 4, 2023

zanettin commented Feb 4, 2023

zanettin commented Feb 5, 2023

B4

nnelgxorz commented Feb 6, 2023

shairez commented Feb 6, 2023

adamdbradley commented Feb 6, 2023

zanettin commented Feb 6, 2023

manucorporat commented Feb 8, 2023

zanettin commented Feb 3, 2023 •

edited

zanettin Feb 3, 2023 •

edited

shairez commented Feb 4, 2023 •

edited