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

refactor: make debugger nullable #12687

Merged
merged 4 commits into from Apr 5, 2023
Merged

refactor: make debugger nullable #12687

merged 4 commits into from Apr 5, 2023

Conversation

bluwy
Copy link
Member

@bluwy bluwy commented Apr 1, 2023

Description

Often times we do:

isDebug && debug('...')

const start = isDebug ? performance.now() : 0

Where isDebug is process.env.DEBUG, however that doesn't always mean the debug will run if DEBUG="blabla" and we're logging with the vite:time namespace.

To simplify this, the PR refactors as debug?.('...') instead.

Additional context

Might make debug logs slightly more accurate.


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

@bluwy bluwy added the p1-chore Doesn't change code behavior (priority) label Apr 1, 2023
@stackblitz
Copy link

stackblitz bot commented Apr 1, 2023

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

sapphi-red
sapphi-red previously approved these changes Apr 1, 2023
Copy link
Member

@sapphi-red sapphi-red left a comment

Choose a reason for hiding this comment

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

LGTM

Maybe returning Debugger | undefined from createDebugger makes it more simple?
Then, we can write debug?.('something') instead of debug.enabled && debug('something').

@bluwy
Copy link
Member Author

bluwy commented Apr 1, 2023

Not sure why I didn't think of that, that feels simpler too 🤔

@bluwy bluwy changed the title refactor: extend debugger with enabled prop refactor: make debugger nullable Apr 1, 2023
@bluwy
Copy link
Member Author

bluwy commented Apr 1, 2023

Updated. I like this new pattern instead, it makes all debug logs lazy by default now.

@bluwy bluwy merged commit 89e4977 into main Apr 5, 2023
14 checks passed
@bluwy bluwy deleted the debugger-refactor branch April 5, 2023 02:22
@patak-dev
Copy link
Member

+1 for the new pattern 💚

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p1-chore Doesn't change code behavior (priority)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants