feat: Transform class integrations into functional integrations #21
Conversation
|
Hmm, actually, thinking about this - maybe we need to split this up in two transforms? 😬 one that does the functional change, and one that removes the integrations package - as the latter will only work on v8 (not on v7)? We don't have such a case yet, so far all transforms work on v7... |
| @@ -0,0 +1,66 @@ | |||
| import { BrowserTracing, Integrations } from '@sentry/browser'; | |||
There was a problem hiding this comment.
I don't remember if we supported this previously anywhere but theoretically, users could also just import { BrowserTracing as MyBrowserTracing } from '@sentry/browser.
In case we currently don't handle this, I think it's also totally fair to ignore this for now. (Even if we say we want to handle this, feel free to open a follow up PR).
There was a problem hiding this comment.
yeah, this will not work with renamed imports for now! We can look at this later if needed :)
| const tracingImportPaths = rewriteEsmImports(SENTRY_REPLAY_PACKAGE, options.sentry.sdk, root, j); | ||
|
|
||
| // 2. Dedupe imports | ||
| if (tracingImportPaths.length > 0) { | ||
| dedupeImportStatements(options.sentry.sdk, root, j); | ||
| } | ||
|
|
||
| // 3. Replace requires | ||
| rewriteCjsRequires(SENTRY_REPLAY_PACKAGE, options.sentry.sdk, root, j); |
There was a problem hiding this comment.
l: Do we need CJS rewrites for replay (considering it's client-side)? I mean I guess it doesn't hurt us either so why not 😅
There was a problem hiding this comment.
Just noticed this while working on this, I think it's OK to just have it here as well 😅
This does a few things related to integrations:
Integrationshash (e.g.new Sentry.Integrations.Http())I wrote a lot of test cases here, hopefully covering all the important stuff...