Skip to content
This repository has been archived by the owner on Nov 23, 2017. It is now read-only.

Incorporate HTTP inspector #213

Closed
linclark opened this issue Aug 11, 2016 · 5 comments
Closed

Incorporate HTTP inspector #213

linclark opened this issue Aug 11, 2016 · 5 comments
Milestone

Comments

@linclark
Copy link

Depends on #195 and #196

In the current console, when you have an HTTP request, you can see the details for that request using a toggle. Most of this code is already React, so it should just require wiring it up.

@linclark linclark added this to the phase 1, sprint 4 milestone Aug 11, 2016
@linclark
Copy link
Author

@rickychien if you're looking for a good next issue, this should be a good one.

@linclark linclark modified the milestones: backlog, phase 1, sprint 5 Sep 27, 2016
@linclark
Copy link
Author

I'm going to deprioritize this. Implementing this might introduce some performance issues in the short term, and we have more critical issues.

@rickychien
Copy link

rickychien commented Sep 28, 2016

@linclark What kind of performance issues you see for implementing HTTP inspector?

BTW, I did a survey and had an early stage WIP patch. I think it's a pretty challenging issue so far and there would be lots of code refactoring in that issue. Such as breaking up net-request.js into action, reducer and selector since net-request.js right now seems like a workaround to connect react NetInfoBody component with current HTML webconsole. Maybe we can create an additional net-request as a container component to be in charge of all network requests.

@janodvarko do you have any idea how to integrate NetworkEventMessage and NetInfoBody?

@linclark
Copy link
Author

linclark commented Sep 28, 2016

@rickychien Thanks for doing that research. To be honest, I haven't looked deeply into it. The performance issue that I foresee has to do with #313. Basically, if we dispatch for every network event update that comes in, we end up going through React's render cycle 9 times for each network message (instead of just once).

This could potentially keep us from landing #283, but I thought we might be able to do a short-term hack to land that one... only dispatching when the updateType was responseHeaders. But I imagine that that short-term hack wouldn't work if we have the http inspector stuff.

Ultimately, I believe most of this info (if not all of it) is available through network monitor, so I don't think that it's as high priority as something like #265

@linclark
Copy link
Author

linclark commented Oct 5, 2016

This will land in m-c in Bug 1307927

@linclark linclark closed this as completed Oct 5, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants