-
Notifications
You must be signed in to change notification settings - Fork 198
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
Improve debugging discoverability #2529
base: main
Are you sure you want to change the base?
Conversation
@@ -12,7 +13,59 @@ registerLogger.formatArgs = function (args) { | |||
originalFormatArgsFunction.call(this, args); | |||
}; | |||
|
|||
const topLevelLogger = registerLogger('teamsJs'); | |||
// eslint-disable-next-line @typescript-eslint/no-unused-vars | |||
const createDebuggerFunction = (namespace: string): Debugger => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is cool and it will probably need a good dose of explanatory comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please take a look on comments? Basically I just explained the code in detail.
2e98a63
to
cf18306
Compare
This looks neat -- can you say more about the use-case for it? Is it something we expect customers might find easier to enable than the current set of logging steps? Or is it something you expect to be able to use when debugging issues? Etc.? |
Based on our discussion with Saurabh, I think the use-case would be customers find the bug/error/failure in their code from Teams-Js library and they wish to know what happened. Then they can read the documentation of |
Need to turn it back to draft again because after rebasing on main, the console log complains that it cannot find timer, which is used by |
For more information about how to contribute to this repo, visit this page.
Description
This is a PR to improve developers debugging experience.
In the past, developers need to turn on client logging following this document so that they can see the logs after app gets refreshed. It will request developers to select correct
iframe
, setverbose
log level, typelocalStorage.debug = 'teamsJs.*'
in console and then refresh the app to get logs from debugger.With this PR, developers only need to set log level in browser as 'verbose' to see logs displayed by debugger. And they can write code as
logger.turnOnDebugLogs()
andlogger.turnOffDebugLogs()
to display the logs from debugger in a segment of code they are interested in.One of use will be put
logger.turnOnDebugLogs()
beforeapp.initialize()
API call so that all of logs during initialization will be displayed on browser console.logger.turnOnDebugLogs()
andlogger.turnOffDebugLogs()
are to set a global variable to be true and false. With this global value, we created a wrapper over original debugger from debug npm library as topLevelLogger. If developers keep following our wiki, it will work as it is and we use debugger directly to display logs. Otherwise, if developers use API call to turn on debug logs, we callconsole.timeLog
to display logs.However, the experience of developers will be the same!
Main changes in the PR:
Validation
Validation performed:
Unit Tests added: No
End-to-end tests added: No