-
Notifications
You must be signed in to change notification settings - Fork 47
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
chore(components): update eslint
& prettier
setup
#2757
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The proposed changes in terms of setup look good overall, aligning us with the ember blueprint.
The main reason I'm not in a position to approve yet, is the prettier
bump – 3.5 is more opinionated on Sass to a point where a single change (the 'Break before breaking comma separated values', poorly implemented in my opinion) results in things like inline comments being misplaced, messing up the readability of the code. And it doesn't seem to provide a way to disable this rule via config.
I'm going to highlight a few such changes that I'm not fond of:



I appreciate prettier 3.5 also comes with support for TypeScript configuration file, which is great, so I see the tradeoffs we'll have to make.
I will bring this PR to engineers attention during our sync today and aim to come up with a resolution.
e19eb87
to
f8262e7
Compare
f8262e7
to
3bca98a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried my best to preserve those inline comments in CSS via prettier config to no avail, so I re-positioned them manually in 3bca98a in a way that hopefully makes sense to everyone. Comments are now either before the line or in line, never after the line.
In regards to the eslint errors, I disabled them in 9a824a3 and captured them in HDS-4656 to facilitate the upgrade preventing us from introducing new errors.
f66121b
to
3e1bd63
Compare
9698bc9
to
c16aea4
Compare
Can we find a way (eg. open a PR/request/issue upstream) to avoid messing up with the Sass format, in particular the comments? For a CSS developer comments deliver a lot of meaning to the code (and their location too). The alternative for me is to have a different automation for Sass files that is less opinionated/restrictive/destructive. |
I can prettier ignore all scss files for now |
📌 Summary
It addresses two things
🛠️ Detailed description
📸 Screenshots
🔗 External links
Jira ticket: HDS-XXX
Figma file: [if it applies]
👀 Component checklist
💬 Please consider using conventional comments when reviewing this PR.