Recommend <!DOCTYPE html> to avoid quirks mode.
jakearchibald
All 14 comments
+9001 to this. It鈥檚 surprisingly common to still see web pages without one.
mathiasbynens
on 15 May 2018
Hey there @jakearchibald, are their some docs I can use to learn how to contribute a new audit to lighthouse? Would love to work on this one. Thanks!
schalkneethling
on 15 May 2018
README.md and CONTRIBUTING.md have some info, and it might help to look at an existing audit that just checks for stuff in the DOM. Here鈥檚 an example: https://github.com/GoogleChrome/lighthouse/blob/master/lighthouse-core/audits/seo/hreflang.js
For this particular audit, you could just check if document.doctype is not null.
mathiasbynens
on 15 May 2018
if there's a document.doctype, looks like you probably also want to check that document.doctype.name is 'html'?
For this you're going to need a gatherer and an audit. Generally the gatherer will collect the information on the doctype from the browser and the audit will do the actual correctness testing. hreflang is a pretty complicated audit compared to this :) so a better example might be the meta-description audit.
The PR for that audit (#3227) lays out pretty well how to get a gatherer and audit to work together, how to add to them to a config, unit tests needed, etc (though most of the SEO category stuff won't be necessary...it would just go into Best Practices in the default config).
Let us know how things go!
brendankenny
on 15 May 2018
Thanks for all of the input everyone. I am going to dig into this. Will let you know should I run into any problems.
schalkneethling
on 16 May 2018
@mathiasbynens @brendankenny Would this audit go inside dobetterweb? Thanks!
schalkneethling
on 18 May 2018
I do not see the following file in the current codebase so, I am guessing a file such as this is no longer required?
https://github.com/GoogleChrome/lighthouse/pull/3227/files#diff-4ba93e471ddd053783bd7729348f18cd
Because this line already exists https://github.com/GoogleChrome/lighthouse/pull/3227/files#diff-37b42887961ae2bfa58b7e227cf7a672R229 I probably do not need to add an entry in this file either? Thanks in advance.
schalkneethling
on 18 May 2018
Yes, fs.readdirSync(path.join(__dirname, './audits/dobetterweb')).map(f => 'dobetterweb/${f}') should read all files in /dobetterweb automatically.
midzer
on 18 May 2018
For requiredArtifacts inside the meta function, passing Doctype is clearly not correct. I poked around and some of the other audits but, they use a variety of values here. What would be the correct one to use for this audit? Thanks!
schalkneethling
on 18 May 2018
Would it help if I opened a WIP PR so folks can see whether I am going in the right direction? @midzer
schalkneethling
on 18 May 2018
@schalkneethling I have not implemented an audit for lighthouse yet, just a minor contribution. Would have to dig deep into this one as well, you've been first to answer this open issue.
I think thats more a question to regular contributors/maintainers if they assist you in a WIP PR.
midzer
on 18 May 2018
@brendankenny Thoughts?
schalkneethling
on 18 May 2018
@schalkneethling yeah go ahead and open up a PR and we can discuss impl details there :)
patrickhulce
on 18 May 2018
Opened the PR: https://github.com/GoogleChrome/lighthouse/pull/5274
When running the test I get the following which to me suggests that what I am passing to requiredArtifacts is not what I should be passing. There might very well be other issues as well. Your assistance is appreciated. Excited to get to know how this all fits together:
$ tsc -p .
lighthouse-core/audits/dobetterweb/doctype.js(15,5): error TS2322: Type '{ name: string; description: string; failureDescription: string; helpText: string; requiredArtifa...' is not assignable to type 'Meta'.
Types of property 'requiredArtifacts' are incompatible.
Type '"Doctype"[]' is not assignable to type '("Manifest" | "URL" | "settings" | "LighthouseRunWarnings" | "fetchTime" | "UserAgent" | "traces"...'.
Type '"Doctype"' is not assignable to type '"Manifest" | "URL" | "settings" | "LighthouseRunWarnings" | "fetchTime" | "UserAgent" | "traces" ...'.
lighthouse-core/audits/dobetterweb/doctype.js(31,19): error TS2339: Property 'Doctype' does not exist on type 'Artifacts'.
lighthouse-core/audits/dobetterweb/doctype.js(37,19): error TS2339: Property 'Doctype' does not exist on type 'Artifacts'.
lighthouse-core/gather/gatherers/dobetterweb/doctype.js(13,36): error TS2339: Property 'Doctype' does not exist on type 'Artifacts'.
lighthouse-core/gather/gatherers/dobetterweb/doctype.js(18,19): error TS2339: Property 'getDocument' does not exist on type 'Driver'.
lighthouse-core/gather/gatherers/dobetterweb/doctype.js(18,38): error TS7006: Parameter 'node' implicitly has an 'any' type.
lighthouse-core/scripts/update-report-fixtures.js(18,91): error TS2339: Property 'port' does not exist on type 'string | AddressInfo'.
Property 'port' does not exist on type 'string'.
Files: 343
Lines: 106462
Nodes: 454373
Identifiers: 154460
Symbols: 165501
Types: 61685
Memory used: 233999K
I/O read: 0.08s
I/O write: 0.00s
Parse time: 1.08s
Bind time: 0.46s
Check time: 2.88s
Emit time: 0.00s
Total time: 4.42s
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
npm ERR! Test failed. See above for more details.
Related issues
mohanrohith
路
3Comments
etelai
路
3Comments
mjara74
路
3Comments
codepodu
路
3Comments
johnfrancisli
路
3Comments
Most helpful comment
if there's a
document.doctype, looks like you probably also want to check thatdocument.doctype.nameis'html'?For this you're going to need a gatherer and an audit. Generally the gatherer will collect the information on the doctype from the browser and the audit will do the actual correctness testing.
hreflangis a pretty complicated audit compared to this :) so a better example might be themeta-descriptionaudit.The PR for that audit (#3227) lays out pretty well how to get a gatherer and audit to work together, how to add to them to a config, unit tests needed, etc (though most of the SEO category stuff won't be necessary...it would just go into Best Practices in the default config).
Let us know how things go!