sitespeedio / plugin-lighthouse

Lighthouse plugin for sitespeed.io
MIT License
28 stars 19 forks source link

Lighthouse parameter passed as string instead of boolean #81

Open joshuagibeonm opened 3 years ago

joshuagibeonm commented 3 years ago

I want to run lighthouse in desktop settings, but based on issue #68, looks like we can't use official documentation guide to run lighthouse in desktop settings. I tried in 16.10.3 but it produce same result as #68.

I tried to dig lighthouse configuration and find that we need to change formFactor and some other settings to run lighthouse in desktop settings. But when I tried to run this command:

docker run sitespeedio/sitespeed.io:16.10.3-plus1 -n 1 --mobile false --lighthouse.extends 'lighthouse:default' 
--lighthouse.settings.formFactor desktop --lighthouse.settings.screenEmulation.mobile false 
'https://www.google.com/' --plugins.add /lighthouse

I got following error:

ERROR: Lighthouse could not test https://www.google.com/ please create an upstream issue: https://github.com/GoogleChrome/lighthouse/issues/new?template=Bug_report.md 
Error: Screen emulation mobile setting (false) does not match formFactor setting (desktop). 
See https://github.com/GoogleChrome/lighthouse/blob/master/docs/emulation.md

I tried to pass verbose flag and this is the output:

...
"lighthouse": {
      "extends": "lighthouse:default",
      "settings": {
        "formFactor": "desktop",
        "screenEmulation": {
          "mobile": "false"
        }
     }
},
...

The flag is passed as string instead of boolean.

I'm not sure if this is the root cause of the error because mobile parameter is passed as string and it's works. Maybe this is happened because in lighthouse, there's strict type validation

soulgalore commented 3 years ago

Hi @joshuagibeonm I think the problem is that yargs (that we use as a cli parser) don't know how to parse parameters that are not pre-defined. Can you try and use a congfiguration file? See https://www.sitespeed.io/documentation/sitespeed.io/configuration/#configuration-as-json

Then we could add a special fix for the mobile/desktop in the plugin.

joshuagibeonm commented 3 years ago

It's possible to use configuration file. In fact, currently I use configuration file to configure lighthouse to run in desktop configuration.

In my case, the problem is that there's some parameter that needs to be passed programmatically. So I can't fully use file to configure the lighthouse. But again, it's still possible to create multiple configuration to satisfy my requirement (my plan if there's no immediate fix for this issue).

Here's my configuration file to run lighthouse in desktop mode:

...
"lighthouse": {
      "extends": "lighthouse:default",
      "settings": {
        "formFactor": "desktop",
        "screenEmulation": {
          "mobile": false,
          ...
        }
     }
},
...
soulgalore commented 3 years ago

@joshuagibeonm I've fixed to handle the --lighthouse.settings.screenEmulation.mobile false case, do you have more examples of configurations that you use that do not work?

joshuagibeonm commented 3 years ago

@soulgalore Thank you for the fix, but I still got the same issue:

verbose:

"lighthouse": {
      "extends": "lighthouse:default",
      "settings": {
        "formFactor": "desktop",
        "screenEmulation": {
          "mobile": "false"
        }
   }
},

I tested master branch using this command:

docker run --rm -v /path/to/plugin-lighthouse:/plugin-lighthouse sitespeedio/sitespeed.io:16.9.2 -n 1 
--mobile false --lighthouse.extends 'lighthouse:default' --lighthouse.settings.formFactor desktop
--lighthouse.settings.screenEmulation.mobile false --outputFolder google 'https://www.google.com/' 
--plugins.add /plugin-lighthouse --verbose