1224751 - Make React's perf tools use the right console

Status: RESOLVED FIXED

(DevTools :: General, defect)

Description

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

I'm not really sure how to do this, but it would be _super_ helpful for me right now.

Comment 1

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

https://facebook.github.io/react/docs/perf.html for reference.

Comment 2

[James Long (:jlongster)]

React's perf tools are already included in the dev version. I got it working with the memory tool last week but never found time to show you (not sure how much it helps at a glance anyway, probably takes some time to go through and parse the data).

There was one snag, the oh-so-common console problem. It looks like the `console` that React references is not the normal window console, so `console.table` is not available, which the perf code uses to dump data. I hacked my local build to reference `window.console` and it worked.

We could probably make the browser loader use `window.console` by default (just override that global), but that whole situation seems like a mess and I don't know the implications of that (no stdout I guess).

Comment 3

[James Long (:jlongster)]

I've confirmed that the perf tools are included in the dev version. The problem is that it doesn't get the web `console` instance, so it doesn't have functions like `console.table`. We've ran into this in several other places, and we should just fix it.

Not sure if we should fix it just for this bug (maybe forcing the global `console` to reference the right one by way of BrowserLoader) or resolve this as a duplicate of another bug which is about fixing console references generally (which I'm hoping exists)

Comment 4

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

I have seen this issue with other modules as well.  I think it's fine to change the global in BrowserLoader.  Will send my patch.

Comment 5

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: MozReview Request: Bug 1224751 - Use the window's console in BrowserLoader. r=jlongster

Review commit: https://reviewboard.mozilla.org/r/31643/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/31643/

Comment 6

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=37ce3243533d

Comment 7

[James Long (:jlongster)]

Comment on attachment 8710053 [details]
MozReview Request: Bug 1224751 - Use the window's console in BrowserLoader. r=jlongster

https://reviewboard.mozilla.org/r/31643/#review28431

Seems sensible to me. I'm a little sad we have to hack it, but it makes sense.

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9e771bb631ce

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9e771bb631ce

Comment 10

[Mayur Patil]

[bugday-20160323]

Status: RESOLVED,FIXED -> UNVERIFIED

Comments:
STR: Not clear.
Developer specific testing

Component: 
Name 			Firefox
Version 		46.0b9
Build ID 		20160322075646
Update Channel 	        beta
User Agent 		Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0
OS                      Windows 7 SP1 x86_64

Expected Results: 
Developer specific testing

Actual Results: 
As expected