-
Notifications
You must be signed in to change notification settings - Fork 132
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
🐛 [RUMF-1267] remove last circular dependencies #1577
Conversation
This commit reorganize recorder types moving most of the src/domain/record/types in src/types. I wanted to do this for a long time, because I'm always confused when looking for a specific type of the recorder. Now everything is at the same level.
* It is a bit unusual to have *Param types. Those suffixes have been removed * MouseInteractions and MediaInteractions enums have been renamed to MouseInteractionType and MediaInteractionType to be a bit more conventional. * ViewportResizeDimention have been renamed to ViewportResizeDimension
7cd426d
to
2b97076
Compare
Codecov Report
@@ Coverage Diff @@
## main #1577 +/- ##
=======================================
Coverage 90.75% 90.75%
=======================================
Files 119 119
Lines 4468 4469 +1
Branches 1001 1001
=======================================
+ Hits 4055 4056 +1
Misses 413 413
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
@@ -0,0 +1,236 @@ | |||
import type { TimeStamp } from '@datadog/browser-core' |
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.
I like the split of the types but now that I'm looking at it, it feels a bit weird to have domain/record/types.ts
and /types/record.ts
.
Either we collocate the types to the module or use a types
folder. My personal preference is to collocate the type with the module but it's not a strong opinion.
Wdyt
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.
Agreed, let's collocate types to the module where they are used
bf2085f
to
6e6ac03
Compare
Motivation
Following #1559 and #1567, this PR removes some more circular dependencies, as well as taking the opportunity to clean recorder types a bit.
Those two last depency loops did not have been caught because of an issue with
import/no-cycle
rule. They have been caught by importing the RUM SDK into a small rollup project.
Changes
Testing
I have gone over the contributing documentation.