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

New feature proposal - Add link to opportunities #113

Merged
merged 37 commits into from
Sep 3, 2020
Merged
Show file tree
Hide file tree
Changes from 34 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
39472ab
Added nyc for testint coverage
JuanMaRuiz Oct 26, 2019
1f65651
Added index test
JuanMaRuiz Oct 26, 2019
e95fe41
Added test to check handleOpts method in index file
JuanMaRuiz Oct 27, 2019
da5adc1
Merge branch 'master' of https://github.com/JuanMaRuiz/psi into featu…
JuanMaRuiz Nov 8, 2019
57fd768
Tests improvements
JuanMaRuiz Nov 8, 2019
f8e7cc7
Added test for threshold
JuanMaRuiz Nov 9, 2019
21a1561
Updated some npm dependencies
JuanMaRuiz Nov 9, 2019
ffb8741
Optimizing unit tests
JuanMaRuiz Nov 9, 2019
695a877
Removed commented tests
JuanMaRuiz Nov 19, 2019
1930431
Merge branch 'feature-optimizing-test' of https://github.com/JuanMaRu…
JuanMaRuiz Nov 19, 2019
a05bdfa
Update test/index.test.js
JuanMaRuiz Nov 21, 2019
6a37baa
Update test/output.test.js
JuanMaRuiz Nov 21, 2019
862aa58
Reverted api-response.test
JuanMaRuiz Nov 21, 2019
6ed67cb
Reverted googleapis npm dependency to previous verison
JuanMaRuiz Nov 21, 2019
94fe536
Added proposed change of test definition
JuanMaRuiz Nov 25, 2019
8e8ab81
Revert unnecessary change
JuanMaRuiz Nov 25, 2019
22c9e77
Moved optionsHandler and getTreshold to options-handler. Removed rewi…
JuanMaRuiz Nov 26, 2019
3b56a1a
Moved getReporter to options-handler
JuanMaRuiz Nov 27, 2019
dbda724
Merge branch 'master' of https://github.com/GoogleChromeLabs/psi
JuanMaRuiz Nov 27, 2019
31bbc65
Merge branch 'master' of https://github.com/GoogleChromeLabs/psi
JuanMaRuiz Nov 28, 2019
d546d6c
Merge branch 'master' of https://github.com/JuanMaRuiz/psi
JuanMaRuiz Nov 28, 2019
31473da
Merge branch 'master' of https://github.com/GoogleChromeLabs/psi
JuanMaRuiz Mar 10, 2020
4c65857
Update googlepais to v40.0.0
JuanMaRuiz Mar 10, 2020
f0abcc6
Merge branch 'master' of https://github.com/GoogleChromeLabs/psi
JuanMaRuiz Mar 14, 2020
df6290b
Add new showLinks options
JuanMaRuiz Mar 15, 2020
54dd8ea
Merge branch 'master' of https://github.com/GoogleChromeLabs/psi
JuanMaRuiz Mar 15, 2020
4d30f5f
Merge branch 'master' into add-link-to-opportunity-title
JuanMaRuiz Mar 15, 2020
b4a627e
Revert index file to match with master
JuanMaRuiz Mar 15, 2020
4668585
Merge branch 'master' of https://github.com/JuanMaRuiz/psi
JuanMaRuiz Mar 15, 2020
9ef7f58
Merge branch 'master' into add-link-to-opportunity-title
JuanMaRuiz Mar 15, 2020
2204ebf
Merge branch 'master' of https://github.com/GoogleChromeLabs/psi
JuanMaRuiz Mar 16, 2020
116d867
Merge branch 'master' into add-link-to-opportunity-title
JuanMaRuiz Mar 16, 2020
4bb63f9
Add missed dependency after merge
JuanMaRuiz Mar 16, 2020
6d38bed
Add improvements from code review
JuanMaRuiz Aug 27, 2020
69e0ca6
Fix getTitle method to manage properly titles without links
JuanMaRuiz Aug 28, 2020
2850571
Revert named groups in regexp due is not supported by node v8
JuanMaRuiz Sep 2, 2020
e8edd2c
Add new param to README
JuanMaRuiz Sep 2, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const cli = meow(`
--format Output format: cli|json|tap
--locale Locale results should be generated in
--threshold Threshold score to pass the PageSpeed test
--links Adds link with more info about opportunities

Example
$ psi https://addyosmani.com --strategy=mobile
Expand Down
20 changes: 16 additions & 4 deletions lib/output.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
const sortOn = require('sort-on');
const humanizeUrl = require('humanize-url');
const prettyMs = require('pretty-ms');
const terminalLink = require('terminal-link');
const {getThreshold, getReporter} = require('./../lib/options-handler');
const {getLink} = require('./../lib/string-utils');

function overview(url, strategy, scores) {
return [
Expand Down Expand Up @@ -53,7 +55,16 @@ const labData = lighthouseResult => {
return sortOn(ret, 'label');
};

const opportunities = lighthouseResult => {
const getTitle = (title, description) => {
let output = title;
if (description) {
exterkamp marked this conversation as resolved.
Show resolved Hide resolved
output = terminalLink(title, getLink(description));
}

return output;
};

const opportunities = (lighthouseResult, links) => {
const {audits, categories} = lighthouseResult;
const rules = getRules(categories.performance.auditRefs, 'load-opportunities');

Expand All @@ -63,9 +74,10 @@ const opportunities = lighthouseResult => {
});

const ret = opportunityRules.map(rule => {
const {title, details} = audits[rule.id];
const {title, details, description} = audits[rule.id];

return {
label: title,
label: links ? getTitle(title, description) : title,
value: prettyMs(details.overallSavingsMs)
};
});
Expand All @@ -85,7 +97,7 @@ module.exports = (parameters, response) => {
overview(humanizeUrl(id), parameters.strategy, lighthouseResult),
fieldData(loadingExperience.metrics),
labData(lighthouseResult),
opportunities(lighthouseResult),
opportunities(lighthouseResult, parameters.links),
threshold
));

Expand Down
9 changes: 9 additions & 0 deletions lib/string-utils.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
const getLink = str => {
const containsLink = str.match(/\((?<url>https?:.*)\)/);
Copy link
Collaborator

@exterkamp exterkamp Sep 1, 2020

Choose a reason for hiding this comment

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

Argh. Node 8 doesn't support named capture groups ☹️

We should go back to numbered groups then I suppose.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep 😢 it's not supported in node v8.

I have reverted the changes. Let me know if you prefer another approach.


return containsLink ? containsLink.groups.url.replace('(', '').replace(')', '') : '';
};

module.exports = {
getLink
};
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
"prepend-http": "^3.0.1",
"pretty-ms": "^6.0.1",
"sort-on": "^4.1.0",
"terminal-link": "^2.1.1",
"update-notifier": "^4.1.0"
},
"devDependencies": {
Expand Down
18 changes: 18 additions & 0 deletions test/string-utils.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/* eslint-env mocha */
const {expect} = require('chai');
const {getLink} = require('../lib/string-utils');

describe('stringUtils module', () => {
describe('getLink method', () => {
it('should return the url from a string', () => {
exterkamp marked this conversation as resolved.
Show resolved Hide resolved
const str = `Consider lazy - loading offscreen and hidden images after all critical resources
have finished loading to lower time to interactive. [Learn more](https://web.dev/offscreen-images).`;
expect(getLink(str)).to.eql('https://web.dev/offscreen-images');
});
it('should return an empty string when it does not contain an url', () => {
const str = `Consider lazy - loading offscreen and hidden images after all critical resources
have finished loading to lower time to interactive.`;
expect(getLink(str)).to.eql('');
});
});
});