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

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

Merged
merged 6 commits into from
Jun 21, 2024

Conversation

bkjam
Copy link
Contributor

@bkjam bkjam commented Jan 15, 2024

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:

  • added an optional --viewer.origin cli args for server (Default: https://googlechrome.github.io)
  • added /v1/viewer/origin endpoint that retrieves the viewer.origin value
  • updated the server docs

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

@connorjclark
Copy link
Collaborator

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
Copy link
Collaborator

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

@brendangadd
Copy link

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
Copy link
Collaborator

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

@connorjclark connorjclark changed the title Changes to use CLI args to configure VIEWER_ORIGIN for lighthouse server feat(server): configure viewer origin from args Jun 21, 2024
@connorjclark connorjclark merged commit f6f80cb into GoogleChrome:main Jun 21, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants