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

Incorrect transfer size for redirected script URL #13724

Closed
adamraine opened this issue Mar 7, 2022 Discussed in #13714 · 6 comments · Fixed by #13751
Closed

Incorrect transfer size for redirected script URL #13724

adamraine opened this issue Mar 7, 2022 Discussed in #13714 · 6 comments · Fixed by #13751
Assignees

Comments

@adamraine
Copy link
Member

adamraine commented Mar 7, 2022

Discussed in #13714

Originally posted by gaborkalmar March 2, 2022
Hey ! I'm looking to improve the LH score of a project and in the network tab I'm getting 243kb as the size of the bundle, but on LH it appears as 900kb transfer size, with 500kb unused. How is this calculated ? Where should I even start with this ?

unused-javascript uses the uncompressed size of resources under the column "Transfer size", however other audits such as total-byte-weight use the compressed size under the column transfer size. I'm not sure how many audits list the uncompressed size as the "Transfer size", but this seems incorrect.

Reproduction steps:

IMO, we should rename the column in unused-javascript to something like "Uncompressed size".

@connorjclark
Copy link
Collaborator

unused-javascript uses the uncompressed size of resources under the column "Transfer size"

Does it?

totalBytes: Math.round(transferRatio * unusedJsSummary.totalBytes),

The bug may instead be in estimateTransferSize.

@adamraine
Copy link
Member Author

The bug may instead be in estimateTransferSize

Yeah I think you're right, seems like it is only this one JS bundle that has a huge transfer size. Seems like the redirect is causing causing a problem because the URL of the bundle under unused JS is the initial URL that get's redirected to the actual script.

@adamraine adamraine changed the title Inconsistent usage of compressed/uncompressed size under "Transfer size" Incorrect transfer size for redirected script URL Mar 7, 2022
@adamraine
Copy link
Member Author

DT protocol events use the initially requested URL for embedderName, so we are fetching the wrong network request here:

for (const [url, scriptCoverages] of Object.entries(artifacts.JsUsage)) {
const networkRecord = networkRecords.find(record => record.url === url);

I suppose we could follow the redirect chain to get the correct network request here.

@gaborkalmar
Copy link

gaborkalmar commented Mar 16, 2022

@adamraine @connorjclark
Hey guys, I'm looking into this issue cause it's kind of a priority for us right now.

If I were to fix this.. let's say in the upcoming weeks, propose the change, open a pr.. what do you think the timeline would be until it gets reviewed/merged and included in a release ? Should I invest time into this ?

@adamraine
Copy link
Member Author

If I were to fix this.. let's say in the upcoming weeks, propose the change, open a pr.. what do you think the timeline would be until it gets reviewed/merged and included in a release ? Should I invest time into this ?

Releases happen every ~3 weeks. Merging a PR can vary a lot especially for external contributors.

This may be complex enough where we would want to handle it internally, but you're welcome to open a PR if you have a solution. I have this marked as "needs investigation" because we don't know how widespread this issue is across our audits. Whenever we look for the network request of a redirected script, we will probably run into this issue.

@gaborkalmar
Copy link

Okay gotcha, although I see there is already a pr opened for it so it's not the case anymore. Thanks for the response :)

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

Successfully merging a pull request may close this issue.

4 participants