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(browser): add user context to MonitoringService #24

Open
wants to merge 2 commits into
base: alpha
Choose a base branch
from

Conversation

il-kilo
Copy link
Contributor

@il-kilo il-kilo commented Jul 28, 2023

Description

Added .userContext object to MonitoringService to be able to provide and persist user.

Motivation and Context

Why is this change required?

  • it helps better track user actions, also it reexpose datadog user API, which was hidden before

Checklist

  • This is a breaking change:
    • Yes
    • No
  • Will this release a new version:
    • Yes
    • No

@il-kilo il-kilo requested a review from skamarakas July 28, 2023 06:20
export interface User {
id?: string | undefined;
email?: string | undefined;
name?: string | undefined;
Copy link
Contributor

Choose a reason for hiding this comment

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

name?: string | undefined; === name?: string same applies for above as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch. You are right, this is subtle but it doesn't work like I expect it to work...
Even with function tsc is still ok with smth like that, while in fact it should not be.
Like if I mark argument as optional, and function has only one argument then what I strictly expect is that either this argument is not passed (arguments.length = 0) or it is passed and it is user (arguments.length = 0 && arguments[0] instance of User). But ts actually allows passing undefined as first argument, which is not expected IMHO and should not be allowed. WDYT?

Screenshot 2023-08-07 at 12 34 59

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In other words what I am saying that either as func argument or key in the object there is a way to distinguish between passing value as undefined and not passing value. In object we can use smth like key in object, in function - smth like arguments length. Though, in function you can avoid passing only last arguments anyway, but you still have a way to check this and can count on that...

@il-kilo il-kilo changed the title feat(cli): add user context to MonitoringService feat(browser): add user context to MonitoringService Aug 7, 2023
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.

2 participants