GoogleChrome / lighthouse-ci

Automate running Lighthouse for every commit, viewing the changes, and preventing regressions
Apache License 2.0
6.33k stars 632 forks source link

feat(server): configure viewer origin from args #1004

Closed bkjam closed 1 week ago

bkjam commented 5 months ago

Hi,

I like the solution in #637 which allows us to change the default self-hosted lighthouse report viewer. However, I noticed that the $VIEWER_ORIGIN environment variable works only if you build a custom @lhci/server package. Hence, I made some modifications to configure $VIEWER_ORIGIN for @lhci/server via the CLI args.

I changed the following things:

This may not be the most optimal solution, so I would appreciate any feedback on my approach and code changes. Thank you :)

connorjclark commented 1 month ago

However, I noticed that the $VIEWER_ORIGIN environment variable works only if you build a custom @lhci/server package.

Do you mean that it only works if you host your own LHCI server? As I understand, you don't need to build anything yourself - the package from npm for @lhci/server will work if you run it with VIEWER_ORIGIN env defined.

If you aren't hosting your own LHCI server, I'm a bit confused why you'd want to be hosting your own report viewer. If that's the case can you explain your use case a bit more?

Otherwise, the code looks good, I'm just not sure why its needed yet.

connorjclark commented 2 weeks ago

@bkjam do you have a moment to give some more information (see above comment)?

brendangadd commented 1 week ago

Do you mean that it only works if you host your own LHCI server? As I understand, you don't need to build anything yourself - the package from npm for @lhci/server will work if you run it with VIEWER_ORIGIN env defined.

@connorjclark: We've been working on using our own report viewer deployment as well, so maybe I can shed some light.

The challenge is that process.env.VIEWER_ORIGIN in LhrViewerLink, being client-side, is evaluated at build-time and not at runtime. Looking at the build result from the @lhci/server package, we can see that the default value is unconditional.

$ cat node_modules/@lhci/server/dist/chunks/entry-*.js | grep -oP 'function[^}]*googlechrome.github.io.{40}'
function La(e){let t="https://googlechrome.github.io";window.addEventListener("message",func

This means you have to build @lhci/server yourself if you want to view the LH reports in a non-default location (e.g. a self-hosted viewer). This PR moves resolution of the viewer URL server-side so that it can be set at runtime and the client can look it up via the API.

By the looks of it, @bkjam preserved priority of the build-time env variable, so people who are building their own package with a custom viewer origin should get consistent behaviour. Though I think that means that packages having a build-time origin will silently ignore any server-side configuration.

In any case, this is a feature that my team would use! :)

connorjclark commented 1 week ago

Right... we put the build result on npm. Thank you for the clarification.