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

console.log commands are not shown their code line locations #26788

Closed
ezranbayantemur opened this issue Oct 9, 2019 · 36 comments
Closed

console.log commands are not shown their code line locations #26788

ezranbayantemur opened this issue Oct 9, 2019 · 36 comments
Assignees
Labels
Bug Resolution: Locked This issue was locked by the bot.

Comments

@ezranbayantemur
Copy link

ezranbayantemur commented Oct 9, 2019

After 0.61.x releases on the debug mode console.log commands are not showing their code line locations. They always shown at ... Project Directioy ...\projectName\node_modules\react-native\Libraries\Core\setUpDeveloperTools.js:73 directory.

For example;
Before update:
1_Ot8O3BcmnRF_JkeerVYWfA

After update:
t1 (1)

Is this a bug? If it's not previous version was way more useful for variable, data or etc.. tracking.

Versions:

    "react": "16.9.0",
    "react-native": "0.61.1"

Environment:
[skip envinfo]

@react-native-bot
Copy link
Collaborator

Can you run react-native info and edit your issue to include these results under the React Native version: section?

If you believe this information is irrelevant to the reported issue, you may write `[skip envinfo]` alongside an explanation in your Environment: section.

@elicwhite
Copy link
Member

@motiz88, does this look familiar to any recent changes? Maybe something like framesToPop on console.log?

@motiz88
Copy link
Contributor

motiz88 commented Oct 10, 2019

This is probably due to the way we wrap console methods to add React component stacks. I'm not sure whether Chrome exposes a way to manipulate the call site attribution for logs. cc @bvaughn

@ezranbayantemur
Copy link
Author

I'm not sure why and how it's happening but it would be more better to work like other version.

@theflutterfactory
Copy link

I'm getting the same result in my console with the newer versions of RN.

@bvaughn
Copy link
Contributor

bvaughn commented Oct 10, 2019

This is probably due to the way we wrap console methods to add React component stacks. I'm not sure whether Chrome exposes a way to manipulate the call site attribution for logs. cc @bvaughn

@motiz88 It does not, although I did open a discussion issue about this type of thing:
whatwg/console#163

It got a little attention initially but seems to have fizzled 😞

I'm not sure why and how it's happening but it would be more better to work like other version.

@ezranbayantemur @motiz88 This is a React DevTools v4 feature, so it would be expected to impact RN v61+.

You should be able to disable it in the general settings.

React component authors have often requested a way to log warnings that include the React "component stack". DevTools now provides an option to automatically append this information to warnings (console.warn) and errors (console.error).

Example console warning with component stack added

It can be disabled in the general settings panel:

Settings panel showing "component stacks" option

@acoutts
Copy link

acoutts commented Oct 11, 2019

I'm also experiencing this after upgrading to React Native 0.61.2. Worked fine in 0.60.x

@bvaughn
Copy link
Contributor

bvaughn commented Oct 11, 2019

To be clear, this is 100% expected to happen for v61+ and not for v60 (or earlier) because v61 embeds the new backend from react-devtools-core v4.

I also made a judgement call for the override to be on by default (so it's opt-out rather than opt-in) because (1) people might not be running the DevTools frontend at all and (2) even if they were, there's an initialization race during which any logged errors would not get the appended callstack.

This decision is somewhat controversial. If folks feel that I made the wrong decision by defaulting on, we can talk about it.

@acoutts
Copy link

acoutts commented Oct 11, 2019

Thanks @bvaughn - for those of us on v61+ now, what's the recommended toolset to continue debugging to see line numbers? I'm willing to switch tools if it means I can see the line numbers again.

@bvaughn
Copy link
Contributor

bvaughn commented Oct 11, 2019

See my comment right above where I copied instructions for disabling it?

@acoutts
Copy link

acoutts commented Oct 11, 2019

When I tried to start react-devtools v4, it said: DevTools v4 is incompatible with this version of React so then I tried v3 and it doesn't look like your screenshot there. There's no settings to toggle.

Is this documented anywhere for me to read more and understand the issue rather than me continue to ask questions?

@bvaughn
Copy link
Contributor

bvaughn commented Oct 11, 2019

Does not sound like you are running v61 of React Native if you saw that message. The v4 frontend can only connect and work with the v4 backend, and vice versa.

@acoutts
Copy link

acoutts commented Oct 11, 2019

Hm, I followed the upgrade guide and checked every change in every file, it's definitely on that version as I have fast refresh as well.

$ react-native upgrade
info No version passed. Fetching latest...
warn Specified version "0.61.2" is already installed in node_modules and it satisfies "0.61.2" semver range. No need to upgrade

@ezranbayantemur
Copy link
Author

You should be able to disable it in the general settings.

React component authors have often requested a way to log warnings that include the React "component stack". DevTools now provides an option to automatically append this information to warnings (console.warn) and errors (console.error).
Example console warning with component stack added
It can be disabled in the general settings panel:
Settings panel showing "component stacks" option

I was tracking logs on Google Chrome Debugger, not React Devtools, so there is no way to disable such as you mention it. But anyway, like @acoutts said when I try to devtools I got incompatible errors.

@IceDev-xyz
Copy link

I also use Chrome Debugger tools, It feels simpler... if this decision can be discussed, count my vote for allowing to see logs like before without having to install React DevTools.

@ledzep9012
Copy link

Can we default it to see logs as they were before if React Devtools is not installed?

@bvaughn
Copy link
Contributor

bvaughn commented Oct 15, 2019

My takeaway from this is that the current default is probably not working out very well for folks, so I'll change it (off instead of on). Sorry for the inconvenience.

@acoutts
Copy link

acoutts commented Oct 15, 2019

Thanks @bvaughn. I think most of the confusion comes from most people not using react devtools but just using Chrome (the thing that opens by default when you enable debugging).

@bvaughn
Copy link
Contributor

bvaughn commented Oct 15, 2019

Follow up clarification question for @ezranbayantemur @ledzep9012 etc: Why was the "before" behavior of showing YellowBox.js (already a React Native console override) preferable to the new location of showing the DevTools backend? Neither is the original place the error/warning was logged.

@ledzep9012
Copy link

Follow up clarification question for @ezranbayantemur @ledzep9012 etc: Why was the "before" behavior of showing YellowBox.js (already a React Native console override) preferable to the new location of showing the DevTools backend? Neither is the original place the error/warning was logged.

@bvaughn I was referring to console.log statements

@bvaughn
Copy link
Contributor

bvaughn commented Oct 15, 2019

Gotcha. I misunderstood the screenshots.

DevTools overrides console.log too so that it can suppress logging when shallow re-rendering a component to inspect hooks values (learn more here). This uses the same mechanism as the "component stacks" feature but the purpose is different.

However, this override is deferred until the first time a component is actually inspected (which would never happen if you weren't using the DevTools UI). It's also unpatched as soon as we're done inspecting. (See the source code here.)

So I'm now unsure of how the originally reported issue was being triggered. Are we actually seeing console.log being overridden eagerly? Are we potentially talking about console.trace instead?

@gaearon
Copy link
Collaborator

gaearon commented Oct 15, 2019

I think there’s a misunderstanding.

Could it be that console.logs are being eaten by the thing that sends them to Metro? So then it wouldn’t be React DevTools-related at all.

In that case ideally the fix would be to somehow detect if we’re in a “rich console” environment like Chrome or not. When we are, don’t hijack console to send logs to Metro.

A possible way to detect that could be to check whether console methods are native or RN polyfill. Maybe by toString-ing them. Or by checking for some Chrome specific property on console. Need to make sure the fix doesn’t give a false positive for Hermes.

@bvaughn
Copy link
Contributor

bvaughn commented Oct 15, 2019

Yes. I assumed this was DevTools because it does a similar override, and I was pinged on this issue. You're right though. It looks like this is a React Native override:

if (!Platform.isTesting) {
const HMRClient = require('../Utilities/HMRClient');
[
'trace',
'info',
'warn',
'log',
'group',
'groupCollapsed',
'groupEnd',
'debug',
].forEach(level => {
const originalFunction = console[level];
// $FlowFixMe Overwrite console methods
console[level] = function(...args) {
HMRClient.log(level, args);
originalFunction.apply(console, args);
};
});
}

Which makes sense, given that the console screenshot above shows setupDeveloperTools.js rather than (DevTools) backend.js

Looks like this change maps back to a recent commit "Improve console logging to Metro":
76e10c4

@cpojer
Copy link
Contributor

cpojer commented Oct 16, 2019

Checking whether console is native or not and skipping the Metro logging setup makes sense to me. Do you wanna send a PR for that?

rickhanlonii added a commit to rickhanlonii/react-native that referenced this issue Oct 16, 2019
Summary:
This diff fixes an issue reported in facebook#26788 where logs in the Chrome console were showing a different location than previous versions.

In the change here, we stop wrapping the console functions to attach the metro websocket in any environment that isn't a native app environment. We do this by checking `global.nativeLoggingHook` which is bound only by native apps and not environments like the Chrome DevTools.

Differential Revision: D17951707

fbshipit-source-id: 64b143c4d256e6ab13150dad2702b12cc4500391
@rickhanlonii
Copy link
Member

This is fixed by #26883

That diff will revert this back to showing "YellowBox.js":

Screen Shot 2019-10-16 at 2 05 08 PM

This will stay the case until Chrome exposes a way for us to ignore our wrapping frame (see here).

Until then, if you want to see the true frame, you can blackbox YellowBox.js (or any script) and Chrome will know to skip that frame:

Screen Shot 2019-10-16 at 2 14 32 PM

Note: The regexes to use are /YellowBox\.js$ and /setUpDeveloperTools\.js$

Screen Shot 2019-10-16 at 2 15 07 PM

Note: Notice that App.js is referenced here instead of YellowBox.js

You can read more about Chrome Blackboxing here

@acoutts
Copy link

acoutts commented Oct 16, 2019

This is fixed by #26883

I just applied this patch and chrome console logs work again for me and it's showing the correct files/ line numbers as before.

Screenshot_2019-10-16_09-37-26

@razirbel
Copy link

razirbel commented Oct 16, 2019

Sorry for my ignorance, but how do you apply the patch? Well for now I just manually changed the file.

@acoutts
Copy link

acoutts commented Oct 16, 2019

Sorry for my ignorance, but how do you apply the patch? Well for now I just manually changed the file.

Manually modify the react native core file in node_modules as per the commit here: 77acfd7

@Dellybro
Copy link

When will the patch fix be merged into the repo?

@gaearon
Copy link
Collaborator

gaearon commented Oct 17, 2019

It will be merged when it's ready. Likely next week.

@ezranbayantemur
Copy link
Author

This is fixed by #26883

Thanks for the support, like @acoutts said this fixed the issue. We waiting for merge.

Closing the issue for fixed, have a great day you all, bugless coding ! 🍺

facebook-github-bot pushed a commit that referenced this issue Oct 21, 2019
Summary:
Pull Request resolved: #26883

This diff fixes an issue reported in #26788 where logs in the Chrome console were showing a different location than previous versions.

In the change here, we stop wrapping the console functions to attach the metro websocket in any environment that isn't a native app environment. We do this by checking `global.nativeLoggingHook` which is bound only by native apps and not environments like the Chrome DevTools.

Changelog: [General][Fixed] - Fix wrong lines logging in Chrome debugger

Reviewed By: cpojer, gaearon

Differential Revision: D17951707

fbshipit-source-id: f045ea9abaa8aecc6afb8eca7db9842146a3d872
@rickhanlonii
Copy link
Member

The fix was merged, we went with a strategy that was a bit better than the original patch tested above so please check it out and let me know if you have any questions 42ac240

@IceDev-xyz
Copy link

0.61.3 - Still shows setUpDeveloperTools.js:73 .. I was with the idea that the fix was already merged for this release, did I miss something?

grabbou pushed a commit that referenced this issue Nov 4, 2019
Summary:
Pull Request resolved: #26883

This diff fixes an issue reported in #26788 where logs in the Chrome console were showing a different location than previous versions.

In the change here, we stop wrapping the console functions to attach the metro websocket in any environment that isn't a native app environment. We do this by checking `global.nativeLoggingHook` which is bound only by native apps and not environments like the Chrome DevTools.

Changelog: [General][Fixed] - Fix wrong lines logging in Chrome debugger

Reviewed By: cpojer, gaearon

Differential Revision: D17951707

fbshipit-source-id: f045ea9abaa8aecc6afb8eca7db9842146a3d872
@tapz
Copy link

tapz commented Dec 4, 2019

0.61.5:

Logcat:

ReactNativeJS: 'Unhandled promise rejection', [TypeError: Cannot read property 'reset' of undefined]

App showing:

console.error: "Unhandled promise rejection', [TypeError: Cannot read property 'reset' of undefined, js engine: hermes

apply

_construct
construct.js:30:26

Wrapper
wrapNativeSuper.js:26:23

call

apply

SyntheticError
index.bundle?platform=andoird&dev=true&minify=false;35761:111

reactConsoleErrorHandler
ExceptionsManager.js:135:52

call

apply

perform$argument_0
....

@tapz
Copy link

tapz commented Dec 4, 2019

index.bundle?platform=andoird&dev=true&minify=false;35761:111 is basically a totally useless line number.

douglowder pushed a commit to react-native-tvos/react-native-tvos that referenced this issue Dec 11, 2019
Summary:
Pull Request resolved: facebook/react-native#26883

This diff fixes an issue reported in facebook/react-native#26788 where logs in the Chrome console were showing a different location than previous versions.

In the change here, we stop wrapping the console functions to attach the metro websocket in any environment that isn't a native app environment. We do this by checking `global.nativeLoggingHook` which is bound only by native apps and not environments like the Chrome DevTools.

Changelog: [General][Fixed] - Fix wrong lines logging in Chrome debugger

Reviewed By: cpojer, gaearon

Differential Revision: D17951707

fbshipit-source-id: f045ea9abaa8aecc6afb8eca7db9842146a3d872
@rickhanlonii
Copy link
Member

@tapz that looks like a different issue, it seems like symbolication is only partially applied.

If you can repro, can you create a new issue?

@facebook facebook locked as resolved and limited conversation to collaborators Oct 3, 2021
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Oct 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

Successfully merging a pull request may close this issue.