report: save to trace-cafe storage for auth-less permalinks #15620

paulirish · 2023-11-17T02:23:49Z

The save as gist workflow has always been complicated.

You always have to go to the viewer webapp.
You see "save as gist" when the feature is really "Get a sharable permalink"..
You authenticate through firebase to talk to... github.?!?! 😖

anyway.. this is a single fetch() POST to upload and no auth needed. 💅

a few moments later:

why https://trace.cafe? I'm using a bucket within the trace-cafe project only because there we already figured out the protected permissions and 365 day TTL stuff. the user doesn't see trace cafe.. heck the URL under the hood is something like https://firebasestorage.googleapis.com/v0/b/tum-lhrs/o?name=lhrs%2Fl<ID>.. but anyway..

This PR drops the upload-as-gist functionality. However apparently we still need firebase/github auth to download gists.. (We really shouldn't, but the PR was getting big already).

Followups:

we can move this feature to the dropdown of all reports (except on file:// protocol). The uploading functionality works everywhere. So from DevTools LH Panel => get a sharable link is very easy now.
we can pipe gist urls through the jsonurl stuff. And that'll let us drop firebase-auth and github-api. (assuming we can get gist content with a single call, no api..)
teach the LH Diff Tool to load from these IDs. Then the pattern for getting a comparison URL is really easy.

paulirish · 2023-11-17T02:28:21Z

viewer/tsconfig.json

@@ -2,7 +2,7 @@
  "extends": "../tsconfig-base.json",
  "compilerOptions": {
    // Limit to base JS and DOM defs.
-    "lib": ["es2020", "dom", "dom.iterable"],
+    "lib": ["es2021", "dom", "dom.iterable"],


needed for string.replaceAll()

connorjclark · 2023-11-17T18:35:41Z

Nice, this is much simpler.

The strings say "permalink", but you mentioned a "365 day TTL". Is this PR currently trading perm-storage for 365 TTL? That seems long enough, but just want to make it clear if that is what's gonna change. If so, calling it "permalink" is not right.

connorjclark · 2023-11-17T18:38:48Z

viewer/app/src/lighthouse-report-viewer.js

-      return id;
+      const updatedUrl = new URL(LighthouseReportViewer.APP_URL);
+      updatedUrl.searchParams.append('id', id);
+      history.pushState({}, '', updatedUrl.href);


wdyt of going even simpler and just making this the jsonurl= to firebase storage URL (then we can drop special handling of ?id. reasons not to:

we like the shorter url

the bucket or path may change

i like the short url. and i'm excited about https://googlechrome.github.io/lighthouse-ci/difftool/?left=<id>&right=<id> Right now with full urls this gets super long and ugggly

viewer/app/src/lighthouse-report-viewer.js

connorjclark · 2023-11-17T18:40:23Z

viewer/app/src/github-api.js

-   * @param {LH.Result|LH.FlowResult} jsonFile The gist file body.
-   * @return {Promise<string>} id of the created gist.
-   */
-  async createGist(jsonFile) {


paulirish · 2023-11-17T18:48:21Z

The strings say "permalink", but you mentioned a "365 day TTL". Is this PR currently trading perm-storage for 365 TTL?

yes. :)

That seems long enough, but just want to make it clear if that is what's gonna change. If so, calling it "permalink" is not right.

this is fair. i realize now i avoided that term on trace.cafe (for that reason) and went with "shareable URL"

Since we're bikeshedding words.. i feel iffy on "Get".. Do you think Get Shareable URL is fine or should we be more explicit that you send your data to do it, eg... Upload for Shareable URL?

connorjclark · 2023-11-17T19:16:09Z

Upload/Get doesn't make much difference to me. You can't get a url without putting your data somewhere. shrug.

But, on the point of "your data", with gist you could always delete your data. afaik you can't with trace.cafe. should that be a requirement before we can use trace cafe? butttt now we need auth again, and you'd need to build out login/deleting data, and .... ugh.

So maybe an explicit prompt when you click "get shareable url" that says "your data will be uploaded to trace.cafe and shareable only via this URL for 365 days. Continue?". With that explicit permissions maybe we can curtail needing to provide a way to delete something

connorjclark · 2023-11-17T19:18:11Z

ID looks like localhost_2021_09_07-70x5q2bg6q3l2mkn4a3oi6m612lo06w63 - so no concern about being able to guess / enumerate all IDs

paulirish · 2023-11-17T20:44:52Z

But, on the point of "your data"

yeah that's where i was coming at it from... i don't like dealing with the permission prompt and storing that preference etc... so... I just renamed it to 'Upload for Shareable URL' to make that action obvious and explicit.

brendankenny · 2023-11-17T21:30:36Z

The user experience seems to be better all around for this (uploading from DevTools is pretty amazing), but as a relatively heavy user of gist uploads, I'll put in a plea for retaining the gist functionality. The biggest thing is probably links working for longer than a year or whatever the TTL ends up settling on. I don't go back through my gists looking for old memories, but I do go back through links from old bugs (year old bugs don't seem as old as they used to), docs, etc and it's definitely nicer when they're still working.

In terms of "your data", I also just generally like the fact that I know where my gists are and how to manage them for things that are meant to be a record of something, vs an ephemeral "hey take a look at this thing" in chat or wherever that trace.cafe is great for.

Since it's an option only visible from the viewer drop down, seems like it could continue living there without adding much confusion about which option to use for getting a shareable link.

adamraine · 2023-11-20T23:17:42Z

we can move this feature to the dropdown of all reports (except on file:// protocol)

Why not file://? Seems like a useful way to share reports generated via CLI.

paulirish · 2023-11-20T23:43:45Z

we can move this feature to the dropdown of all reports (except on file:// protocol)
Why not file://? Seems like a useful way to share reports generated via CLI.

I agree the cli generated reports would really benefit from this feature :)
for some reason i thought you couldn't make http CORS requests from file:// but... i just confirmed they work fine. Okay then nevermind!

paulirish · 2023-11-20T23:46:47Z

I'll put in a plea for retaining the gist functionality.

@brendankenny want gist uploading functionality? cuz i was gonna keep around downloading. also in that world, you can still upload gists manually at gist.github. wdyt?

brendankenny · 2023-11-20T23:53:45Z

@brendankenny want gist uploading functionality? cuz i was gonna keep around downloading.

Yeah, sorry, I meant gist uploading.

also in that world, you can still upload gists manually at gist.github. wdyt?

Well, yes, similar to manually uploading at https://trace.cafe/ :P Having a button makes it very nice :)

adamraine · 2023-11-21T22:15:26Z

viewer/app/src/lighthouse-report-viewer.js

-            : null;
-        })
-        .then(() => fetch(jsonurl))
+    } else if (jsonurl ?? cafeid) {


nit: this is the effective condition so let's make it explicit

Suggested change

} else if (jsonurl ?? cafeid) {

} else if (jsonurl || cafeid) {

adamraine · 2023-11-21T22:19:25Z

viewer/app/src/lighthouse-report-viewer.js

+          finalDisplayedUrl: Util.getFinalDisplayedUrl(reportJson),
+          fetchTime: reportJson.fetchTime,
+        });
+      }


Can this filename logic move inside uploadLhrToTraceCafe?

paulirish added 4 commits November 16, 2023 17:02

report: save to trace-cafe storage

040e0f6

state juggling

0655a72

create id based on SHA-1 of json content

65664c9

title case

2b4304c

paulirish requested a review from a team as a code owner November 17, 2023 02:23

paulirish requested review from connorjclark and removed request for a team November 17, 2023 02:23

minor

cb62bbc

paulirish commented Nov 17, 2023

View reviewed changes

vercel bot deployed to Preview November 17, 2023 02:28 View deployment

devtools-bot assigned connorjclark Nov 17, 2023

devtools-bot added the waiting4reviewer label Nov 17, 2023

minor: strings

3a9b67e

vercel bot deployed to Preview November 17, 2023 02:41 View deployment

connorjclark reviewed Nov 17, 2023

View reviewed changes

viewer/app/src/lighthouse-report-viewer.js Outdated Show resolved Hide resolved

connorjclark reviewed Nov 17, 2023

View reviewed changes

add string to proto

d3da465

rename to 'Upload for Shareable URL'

f379623

vercel bot deployed to Preview November 17, 2023 20:42 View deployment

cafeid

cc43041

vercel bot deployed to Preview November 17, 2023 20:43 View deployment

build report

84cf5e9

vercel bot deployed to Preview November 17, 2023 20:47 View deployment

adamraine reviewed Nov 21, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

report: save to trace-cafe storage for auth-less permalinks #15620

report: save to trace-cafe storage for auth-less permalinks #15620

paulirish commented Nov 17, 2023 •

edited

paulirish Nov 17, 2023

connorjclark commented Nov 17, 2023

connorjclark Nov 17, 2023

paulirish Nov 17, 2023

connorjclark Nov 17, 2023

paulirish commented Nov 17, 2023

connorjclark commented Nov 17, 2023 •

edited

connorjclark commented Nov 17, 2023 •

edited

paulirish commented Nov 17, 2023

brendankenny commented Nov 17, 2023

adamraine commented Nov 20, 2023

paulirish commented Nov 20, 2023

paulirish commented Nov 20, 2023

brendankenny commented Nov 20, 2023

adamraine Nov 21, 2023

adamraine Nov 21, 2023

	} else if (jsonurl ?? cafeid) {
	} else if (jsonurl \|\| cafeid) {

report: save to trace-cafe storage for auth-less permalinks #15620

Are you sure you want to change the base?

report: save to trace-cafe storage for auth-less permalinks #15620

Conversation

paulirish commented Nov 17, 2023 • edited

paulirish Nov 17, 2023

Choose a reason for hiding this comment

connorjclark commented Nov 17, 2023

connorjclark Nov 17, 2023

Choose a reason for hiding this comment

paulirish Nov 17, 2023

Choose a reason for hiding this comment

connorjclark Nov 17, 2023

Choose a reason for hiding this comment

paulirish commented Nov 17, 2023

connorjclark commented Nov 17, 2023 • edited

connorjclark commented Nov 17, 2023 • edited

paulirish commented Nov 17, 2023

brendankenny commented Nov 17, 2023

adamraine commented Nov 20, 2023

paulirish commented Nov 20, 2023

paulirish commented Nov 20, 2023

brendankenny commented Nov 20, 2023

adamraine Nov 21, 2023

Choose a reason for hiding this comment

adamraine Nov 21, 2023

Choose a reason for hiding this comment

paulirish commented Nov 17, 2023 •

edited

connorjclark commented Nov 17, 2023 •

edited

connorjclark commented Nov 17, 2023 •

edited