core(script-treemap-data): create node for each inline script#13802
Conversation
| htmlNode.resourceBytes += node.resourceBytes; | ||
| if (node.unusedBytes) htmlNode.unusedBytes = (htmlNode.unusedBytes || 0) + node.unusedBytes; | ||
| node.name = script.content ? | ||
| '(inline) ' + script.content.trimStart().substring(0, 15) + '…' : |
There was a problem hiding this comment.
Since we are applying the "(inline)" here, could we remove this code in the treemap viewer:
lighthouse/treemap/app/src/main.js
Lines 87 to 89 in 0b0077d
It would allow us to remove the document url dependency from the treemap viewer.
There was a problem hiding this comment.
These lines act on different nodes - the line in main.js appends (inline) to the depth-1 node that has a name which matches the document url, but here it's that node's children's names.
It would allow us to remove the document url dependency from the treemap viewer.
What about all the other usages of url? or do you mean changing to finalUrl as opposed to requestedUrl like the PR you closed did?
There was a problem hiding this comment.
These lines act on different nodes - the line in main.js appends (inline) to the depth-1 node that has a name which matches the document url, but here it's that node's children's names.
But is it still necessary now that all the children have (inline) attached? If we are going to keep it, it would be better to add an isInline flag to the node than do a URL check in the viewer.
What about all the other usages of url? or do you mean changing to finalUrl as opposed to requestedUrl like the PR you closed did?
Yeah sorry, we could change to finalUrl without worrying because all other usages of the url are cosmetic or just looking at the origin. This part doesn't need to be done here, I can tackle it as part of #13706
There was a problem hiding this comment.
Changing it to use finalUrl should be fine. I think the UI here can be confusing if / is showed instead of / (inline) (we elide the origin to make the names shorter, so the final/requestedUrl changes to just /), so I don't want to remove it. And adding a flag "isInline" is something I'd like to avoid if we can.
There was a problem hiding this comment.
Anyway, if we did these changes it'd be better in a separate PR, can you review this PR independent of these proposed changes?
| "serve-dist": "cd dist && python -m SimpleHTTPServer", | ||
| "serve-gh-pages": "cd dist/gh-pages && python -m SimpleHTTPServer", | ||
| "serve-dist": "cd dist && python3 -m http.server", | ||
| "serve-gh-pages": "cd dist/gh-pages && python3 -m http.server", |
There was a problem hiding this comment.
python3 is king now
This will also group the inline scripts by frame, although at the moment we do not collect scripts for anything other than the main frame.
before
after
