Skip to content

Check for console #185

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

Merged
merged 2 commits into from
Oct 28, 2016
Merged

Check for console #185

merged 2 commits into from
Oct 28, 2016

Conversation

vectart
Copy link
Contributor

@vectart vectart commented Oct 27, 2016

It breaks in IE9 when the package is just required.

@codecov-io
Copy link

Current coverage is 9.32% (diff: 100%)

Merging #185 into master will not change coverage

@@            master      #185   diff @@
========================================
  Files            5         5          
  Lines          118       118          
  Methods          0         0          
  Messages         0         0          
  Branches         0         0          
========================================
  Hits            11        11          
  Misses         107       107          
  Partials         0         0          

Powered by Codecov. Last update 781f790...be0447f

Copy link
Collaborator

@grushetsky grushetsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vectart, could you, please, explain why we need that change? How could this problem be reproduced?

@@ -1,6 +1,6 @@
export default {
level: `log`,
logger: console,
logger: (this && this['console']) || (window && window['console']),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vectart, the build fails on linting step. You should use this.console and window.console here.

But anyway, I don't really understand the reason for this change. console object is defined in Internet Explorer 9 by default, isn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Build is fixed

@vectart
Copy link
Contributor Author

vectart commented Oct 28, 2016

@grushetsky Yes, the console is undefined until Developer Panel has been opened http://stackoverflow.com/questions/7742781/why-does-javascript-only-work-after-opening-developer-tools-in-ie-once

@grushetsky
Copy link
Collaborator

@vectart, OK, I see the point, thanks for finding this problem.

Why do we need to check for this.console, isn't window.console enough here?

@vectart
Copy link
Contributor Author

vectart commented Oct 28, 2016

@grushetsky it's for node environment

@grushetsky
Copy link
Collaborator

@vectart, oh, sure. 👌🏻

@grushetsky grushetsky merged commit 0193721 into LogRocket:master Oct 28, 2016
@char1e5
Copy link

char1e5 commented Oct 28, 2016

I got the same error. Have to use 2.7.0 now.
logger: undefined && undefined.console || window && window.console,

@char1e5
Copy link

char1e5 commented Oct 29, 2016

This fix should work on node
#188

@imevro
Copy link
Collaborator

imevro commented Oct 29, 2016

@grushetsky @vectart guys please fix this, I revert that PR for now

imevro pushed a commit that referenced this pull request Oct 29, 2016
@laysent laysent mentioned this pull request Sep 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants