Conversation
size-limit report
|
0dcc288 to
61feeeb
Compare
|
I believe we cannot do this change as we support browsers that do not have the URL object. |
|
@mitsuhiko I reverted the commits that changed parsing, there should probably still be a small decrease in bundle size |
packages/utils/src/dsn.ts
Outdated
| } | ||
| validateDsn(components); | ||
|
|
||
| const dsn: Dsn = { |
There was a problem hiding this comment.
Not a huge fan of returning objects with toString methods directly on it, rather than prototypes. Do we need toString at all? Can we instead make it a helper function?
There was a problem hiding this comment.
I also noticed a test failure, i think the toString function in it is the culprit.
There was a problem hiding this comment.
@mitsuhiko, not a fan either, I didnt want to do too many changes, but since we should be the only ones to consume this, let's try remove it. btw, I already made it a helper fn - the toString that we expose just calls dsntoString 😅
| } | ||
|
|
||
| /** The Sentry Dsn, identifying a Sentry instance and project. */ | ||
| export class Dsn implements DsnComponents { |
There was a problem hiding this comment.
Mind writing a quick summary on how to convert from using the Dsn class to the functions? We’ll need to add it to the changelog for react native and electron.
db2aa7d to
a22532a
Compare
Removed the class constructor and refactored the utility fn to allow for better tree shaking. Instead of using the Dsn constructor, you should now use
makeDsnanddsnToStringequiv to the Dsn.toString() class method that we used before