misc(viewer): support 3.x and legacy (2.x) reports in viewer #5204
Conversation
| if (messageEvent.data.opened) { | ||
| popup.postMessage({lhresults: json}, VIEWER_ORIGIN); | ||
| window.removeEventListener('message', msgHandler); |
There was a problem hiding this comment.
Is this because we need to listen for the render message to close? will this cause duplicate handler problems if they click "Open in viewer" twice?
There was a problem hiding this comment.
good question. @ebidel is that the situation this is handling?
There was a problem hiding this comment.
I remember one issue is if you open viewer from a report, then use viewer as a viewer (looking at other reports or something), if you refresh it will send back an opened message back to the original report and load the first report, not whatever you've been doing since.
There was a problem hiding this comment.
I think what @brendankenny described is the sitch. Although I'm not sure why Chrome does that, but you sometimes get another message event when the page refreshes.
| const viewerPath = '/lighthouse/viewer2x/'; | ||
|
|
||
| logger.log(warnMsg, false); | ||
| features.sendJsonReport(json, viewerPath, onMessage); |
There was a problem hiding this comment.
impl is a bit confusing that we create a temporary instance of the ViewerUIfeatures to trigger the render in viewer, feels a bit hacky
I think it's mostly just the fake flexibility of sendJsonReport that confuses me. WDYT of making this a bit more explicit with a sendJsonReportToV2 or something that both sure the same underlying logic?
they could just resolve a promise when they get confirmation that the report has been rendered and nix the onMessage piece
There was a problem hiding this comment.
generally this looks workable, I'm just wondering if we want another twist of hard-to-follow postMessaging passing through various inheritance levels :)
If we really want to do it right it feels like we should be bundling the report generator, loading the v3 bundle by default, and then loading the v2 bundle if the report is older. Then it should be pretty isolated to _replaceReportHtml(), where it'll just have to be calls to new bundle.DOM(), new bundle.ReportRenderer, and new bundle.ViewerUIFeatures() (or pull the init code into the bundle) and everything else is pretty much just saving/loading reports to pass into the versioned bundle
I tried this but it's real nasty. See https://github.com/GoogleChrome/lighthouse/compare/viewer2x-open-versioned-samedpage and branch2.x...viewer2x-deploy-versioned removeEvtListener restored. we good here? |
Basic setup:
viewer2x/and has it render the report.Screencast:
This also relies on a few commits added to 2.x branch: branch2.x...viewer2x-deploy
fixes #5170