Add --har option to save HAR of network activity #301
Conversation
ghost
added
|
woah this is cool. Nice work Paul! |
|
I'm concerned that this will lead to memory leaks since every single request is cached by the |
|
Can we inline-require it? |
| if (this.flags.har) { | ||
| steps.push(async (curr: number, total: number) => { | ||
| this.reporter.step(curr, total, 'Saving a HAR', emoji.get('recycle')); | ||
| await requestHarCapture.saveHar(`kpm-install_${new Date().toISOString()}.har`); |
There was a problem hiding this comment.
Could we include the name of the saved file in the previous message?
|
@cpojer I'm not sure what you mean? Module loading doesn't really have anything to do with it. @paulirish Actually, could you add |
|
I'm thinking we could only require the capture module when the har flag is passed? |
|
That could work. I'd really prefer if the request HAR module could localise it's state rather than being global. Multiple installs in the same process can't have separate HAR files for example. |
|
Opened paulirish/request-capture-har#2 to isolate the capture state. Let me know what you think @paulirish. |
|
Awesome! Looks like there's a few type errors, |
|
Thanks to @kittens's PR, request-capture-har now keeps its state local. I've now updated this PR:
Other than that, I'm seeing some flow type errors. Might take me a moment to sort that out; never worked with Flow before. |
| @@ -33,6 +33,7 @@ type ConfigOptions = { | |||
| modulesFolder?: string, | |||
| offline?: boolean, | |||
| preferOffline?: boolean, | |||
| captureHar: boolean, | |||
There was a problem hiding this comment.
My bad, looks like this needs a ? like the other properties.
There was a problem hiding this comment.
yup! just tackled. thanks for the help
|
Flow now passing. Not bad at all. :) |
|
This is great! Thank you @paulirish! |
|
Looks like trunk got broken when this PR was merged https://circleci.com/gh/facebook/fbkpm/491 |
I was curious about the network performance of kpm. That ended up here, a new command line flag which can save a HAR file.
Screenshots:
As part of this effort, I contributed a small patch to
requestwhich addsresponseStartTimetiming. As of today, this is not in a shipped version ofrequest, so I've adjusted the dependency. (It's currently pointing a few commits past the recent2.74.1)Already, the data from the HAR is pretty fascinating, though I bet it'd be more useful in y'allz hands.