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

feat: Add global logger object #1222

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

webJose
Copy link

@webJose webJose commented May 12, 2024

Motivation

I personally would like to remove minified error message 1 (see #1161), and others have expressed their desire to completely suppress logging.

With this solution, both are possible.

How It Works

A new utility module named logging has been added to the src/utils folder that exports 2 objects:

  1. The setLogger() function that controls the destination of all log entries.
  2. The proxy logger object, whose methods write to the log destination set by setLogger().

By default, the log destination is the console. Users can, at any point in their application's lifecycle, call setLogger() to control which log entries make it to the browser's console.

setLogger()

This function takes 1 argument that can be false, true or a custom destination log object.

The value false internally sets a log destination that discards all messages; the value true routes all messages to the console. The third option allows the caller to take complete control over what to do with the logged messages.

For Maintainers and Contributors

Never do console.debug(), console.info(), console.warn() or console.error() ever again. Always replace console with logger. Logger is imported:

import { logger } from 'path/to/src/utils/logging.js';

@webJose
Copy link
Author

webJose commented May 12, 2024

@MilanKovacic here's the drafted PR for the global logging feature. It is complete and contains 12 new unit tests. All the new tests are passing.

As seen in the PR, all previous unit testing related to console writing pass without modification, which should be a testament of the neutral nature of the change. This change is fully backwards compatible.

Please review and let me know if any changes are needed.

@MilanKovacic MilanKovacic self-requested a review May 15, 2024 22:28
Copy link

@ibacher ibacher left a comment

Choose a reason for hiding this comment

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

Couple of small things. Thanks @webJose!

src/utils/logging.js Outdated Show resolved Hide resolved
src/utils/logging.js Outdated Show resolved Hide resolved
@webJose
Copy link
Author

webJose commented May 20, 2024

Changes applied!

src/utils/logging.js Outdated Show resolved Hide resolved
@webJose
Copy link
Author

webJose commented May 22, 2024

Changes made. Additionally, I created the corresponding PR in the documentation website.

@ibacher
Copy link

ibacher commented May 24, 2024

@webJose Can you mark this as no longer a draft? @MilanKovacic what do you think about merging this?

@webJose webJose marked this pull request as ready for review May 24, 2024 14:40
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.

None yet

2 participants