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

Add lighthouse scripts #26

Merged
merged 8 commits into from
Jul 29, 2022
Merged

Add lighthouse scripts #26

merged 8 commits into from
Jul 29, 2022

Conversation

tellyworth
Copy link
Contributor

@tellyworth tellyworth commented Jul 28, 2022

This installs lighthouse-ci and adds an easy script for running Lighthouse.

Props ,

Screenshots

Screen Shot 2022-07-28 at 12 16 28 pm

How to test the changes in this Pull Request:

  1. yarn
  2. yarn lighthouse
  3. yarn lighthouse:desktop
  4. yarn lighthouse:mobile

@tellyworth tellyworth added the [Component] Tools Build tools, packages, etc label Jul 28, 2022
Copy link
Contributor

@ryelle ryelle left a comment

Choose a reason for hiding this comment

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

Works well! Just needs a rebase to fix the conflicts, and the npm commands aren't needed in the package.json scripts.

package.json Outdated
Comment on lines 22 to 24
"lighthouse": "npm exec -- lighthouse-ci http://localhost:8888/ --accessibility=100 --best-practices=100",
"lighthouse:desktop": "npm exec -- lighthouse http://localhost:8888/ --view --preset=desktop --output-path=lighthouse.html",
"lighthouse:mobile": "npm exec -- lighthouse http://localhost:8888/ --view --preset=mobile --output-path=lighthouse.html"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"lighthouse": "npm exec -- lighthouse-ci http://localhost:8888/ --accessibility=100 --best-practices=100",
"lighthouse:desktop": "npm exec -- lighthouse http://localhost:8888/ --view --preset=desktop --output-path=lighthouse.html",
"lighthouse:mobile": "npm exec -- lighthouse http://localhost:8888/ --view --preset=mobile --output-path=lighthouse.html"
"lighthouse": "lighthouse-ci http://localhost:8888/ --accessibility=100 --best-practices=100",
"lighthouse:desktop": "lighthouse http://localhost:8888/ --view --preset=desktop --output-path=lighthouse.html",
"lighthouse:mobile": "lighthouse http://localhost:8888/ --view --preset=mobile --output-path=lighthouse.html"

@StevenDufresne
Copy link
Contributor

If we wanted, we could also run the tests on PR or trunk merge and save the artifact within the action.

@tellyworth
Copy link
Contributor Author

Yeah I think lighthouse-ci will be useful for that. It's probably not needed at this stage but I might make a draft PR after merging this part.

Also remove redundant `npm exec`
@tellyworth tellyworth merged commit fc80ca4 into trunk Jul 29, 2022
@tellyworth tellyworth deleted the add/lighthouse branch July 29, 2022 04:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Component] Tools Build tools, packages, etc
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants