Skip to content

Don't output console.logs in production (don't use console.log directly) #565

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

Closed
coopy opened this issue May 24, 2016 · 6 comments · Fixed by #590
Closed

Don't output console.logs in production (don't use console.log directly) #565

coopy opened this issue May 24, 2016 · 6 comments · Fixed by #590
Labels
community community contribution

Comments

@coopy
Copy link

coopy commented May 24, 2016

Plotly.js uses raw console.log statements to print info, warning, and error messages. Instead, it should use an abstract logger so we can silence warnings in production. Something like https://github.com/FormidableLabs/abstract-log that lets us configure a null transport in production for anything but errors.

As an example of this happening, try typing some HTML into the rich annotations editor. Before you've closed the opening tag, svg_text_utils.js spews errors for every update.

open-tag

@etpinard etpinard added the community community contribution label May 25, 2016
@etpinard
Copy link
Contributor

And we should probably add the no-console eslint rule to the src .eslintrc.

@n-riesco
Copy link
Contributor

I was thinking of submitting a PR to replace all the occurrences of console.log under src/ with lib.log. Would that be OK?

@etpinard
Copy link
Contributor

etpinard commented May 25, 2016

@n-riesco I think we should at least expose a config argument so that users that want to see the logs can see them, using:

Plotly.plot('graph', data, layout, { verbose: true });

@mdtusz
Copy link
Contributor

mdtusz commented May 25, 2016

Agreed on this - probably would be good to swap most console.log statements to Lib.log, but I still think there are some true errors/warnings that should remain (e.g. things like calling Plotly.plot with invalid arguments).

@etpinard
Copy link
Contributor

but I still think there are some true errors/warnings that should remain

maybe throwing errors would be better.

@mdtusz
Copy link
Contributor

mdtusz commented May 25, 2016

😑 ...

Yep.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community community contribution
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants