Comments (9)
I'm a BIG proponent of an auto-formatter, too.
https://github.com/angular/clang-format is one direction we could go.. now that it does support JS.
from lighthouse.
I'm mostly okay with the Google style. My only pref is to stick to 80 chars. Longer than that tends to unreadable code.
from lighthouse.
#5 adds base Google style eslint checking. A couple of issues to address. Biggest thing is standard
style putting a space before the parameter list and some excess white space. I'm happy to go through the current errors and do a quick auto-fix for those.
There are a handful of things that may need some bikeshedding.
from lighthouse.
Remaining eslint warnings for current code:
- Line length. Maybe 100 characters as a compromise? I personally like a line limit but I don't care very much how long it is.
- TODOs and FIXMEs. At first I was just going to remove this rule, but it is kind of handy to have something to nag us if we're going to leave them in. Bugs planned to be long term should really be in issues, not in the code. On the other hand: annoying. Thoughts?
- Missing JSDoc comments. Looks like maybe if you start JSDocing a file eslint wants you to finish, since this warning only shows up in one file. Or maybe it's a es5/es2015 thing? Obviously this is a larger conversation re: doc strings and building tools, so happy to suppress (or not) this warning for now.
from lighthouse.
investigated clang-format today. For posterity's sake, you can recreate my results with something like npm install --save-dev clang-format
(includes Win, Linux, and Mac binaries), creating a .clang-format
file with something like:
---
Language: JavaScript
BasedOnStyle: Chromium
ColumnLimit: 100
AllowShortFunctionsOnASingleLine: Empty
TabWidth: 2
UseTab: Never
and then run with node_modules/.bin/clang-format -i --glob='{,./!(node_modules)/**/}*.js'
output is...OK. It does some screwy things with object and array literals and arrow functions, sometimes putting lots of things on one line and sometimes not. There may be an additional rule to fix those that I'm missing, but I couldn't find it. .then
s it really likes to put on one line if not using an inline function, which is something I'm not wild about either.
It's nice to have a tool that fixes a bunch of style issues automatically, but not sure if it's worth it.
from lighthouse.
I'd prefer sticking with something like .eslintrc, just because that's what the majority of people use. Ideally people from the community can and should contribute as they wish, and the less ... unusual the better imo.
from lighthouse.
from lighthouse.
yeah, just to be clear, the hope was to be able to configure clang-format so that it would fix violations of the exact same rules as in the .eslintrc file, so it would be an option for those who e.g. need a robot to hit the semicolon key for them.
Looks like clang-format is too pushy on other fronts for that to work out, though.
from lighthouse.
- Let's proceed with line limit of 80. :)
- TODOS and FIXMEs will remain warnings. seems good.
groovy. i think we're set.
from lighthouse.
Related Issues (20)
- Unexpected changes in main thread time reported around March 8th HOT 8
- Sequential network requests are simulated as parallel and the first rtt is ignored
- Lone surrogate character breaks PSI proto conversion HOT 4
- Was filing bug report when site crashed multiple before I could get the first report in HOT 2
- On certain audited pages, `totalBodyElements` are different from `MainDocumentContent` body nodes count HOT 1
- The Future of WordPress in the Age of AI Integration HOT 2
- Site security issues while making payment to Comcast. HOT 1
- SEO updates from recent search conversations
- Unexpected changes on main thread work from 11.6.0 to 11.7.1 HOT 3
- Viewport resizing to window size
- Lighthouse LCP Issue HOT 1
- LCP not recognized in many pages for this site. HOT 1
- Print stylesheet picked for Render-blocking resource HOT 1
- Incorrect user agent on pagespeed.web.dev HOT 1
- Protocol error (Target.getTargetInfo): Protocol error (Target.getTargetInfo): 'Target.getTargetInfo' wasn't found HOT 3
- Keep passing Core Web Vitals but failing LCP
- "Opportunities" no longer surfaced in report or in resulting output in Lighthouse 12 HOT 5
- CPU throttling is unstable
- Testing Page metrics for different HTTP versions HOT 1
- LH:ChromeLauncher Waiting for browser
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from lighthouse.