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

fix(scss): improve error logs #18522

Merged
merged 5 commits into from
Oct 31, 2024
Merged

fix(scss): improve error logs #18522

merged 5 commits into from
Oct 31, 2024

Conversation

bluwy
Copy link
Member

@bluwy bluwy commented Oct 30, 2024

Description

There's 3 commits in this PR:

  1. Add e.line and e.column to error object when compiler with scss modern api. It's missing by default which makes the file name open-in-editor not have the line/col. (Astro had a test that expected the positional info to exist. I don't think this is a breaking change, but figured nice to fix anyway)
  2. (and 3) I fiddled with making the error overlay look better because with no1, modern api error will cause Vite to regenerate a code frame, causing two code frames to appear. At the end, I shimmed e.frame = e.message to prevent it (mimicking same behaviour as legacy api for now)

I think the error messages could be a little better for css preprocessors, but it's a bit subjective so i didn't want to make a too drastic change for now.

Before (legacy) image
Before (modern) image
After (legacy) image
After (modern) image

patak-dev
patak-dev previously approved these changes Oct 30, 2024
hi-ogawa
hi-ogawa previously approved these changes Oct 31, 2024
Copy link
Collaborator

@hi-ogawa hi-ogawa left a comment

Choose a reason for hiding this comment

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

Nice!

packages/vite/src/node/plugins/css.ts Outdated Show resolved Hide resolved
@sapphi-red sapphi-red added feat: css p2-edge-case Bug, but has workaround or limited in scope (priority) labels Oct 31, 2024
Co-authored-by: Hiroshi Ogawa <hi.ogawa.zz@gmail.com>
@sapphi-red sapphi-red dismissed stale reviews from hi-ogawa and patak-dev via 9dd36f0 October 31, 2024 02:07
@sapphi-red sapphi-red enabled auto-merge (squash) October 31, 2024 02:10
@sapphi-red sapphi-red merged commit 3194a6a into main Oct 31, 2024
15 checks passed
@sapphi-red sapphi-red deleted the scss-line-col branch October 31, 2024 02:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat: css p2-edge-case Bug, but has workaround or limited in scope (priority)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants