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

core(runner): fix missing timing properties in the browser #9072

Merged
merged 3 commits into from
May 28, 2019

Conversation

exterkamp
Copy link
Member

Summary
PerformanceEntries in Lightrider (and the extension) had no names and no entryTypes. This manifested as a 0 timing.total in the API as well.

@brendankenny found that browser PerformanceEntry objects do not spread, but in Node they do b/c of the marky lib. So, simple fix to explicitly call out the name and entryType fields when parsing the entries. This will also populate the missing lh:runner:run entry to fill in timing.total.

Related Issues/PRs
fixes: #8638

@brendankenny brendankenny changed the title core(runner): fix PerformanceEntry names and entryTypes core(runner): fix missing timing properties in the browser May 28, 2019
@brendankenny
Copy link
Member

@brendankenny found that browser PerformanceEntry objects do not spread, but in Node they do b/c of the marky lib

marky makes its own PerformanceEntry objects, which spread just fine, but the properties on a native PerformanceEntry are getters in the prototype chain, so nothing is spread.

Interestingly, running LH in DevTools seems to defeat marky's environment sniffing and so it uses the fallback objects and the entries are ok. The extension is like lr here and only has {startTime, duration}

Copy link
Collaborator

@patrickhulce patrickhulce left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bam nice work! with 1.5 quarters to spare too 😎

lighthouse-core/runner.js Show resolved Hide resolved
@connorjclark
Copy link
Collaborator

can we add a hexa test to LR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Total timing is equal to 0 always via API
6 participants