Skip to content

feat(core): Fix legacy integration class types & export integration functions (WIP) #10143

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

Closed
wants to merge 1 commit into from

Conversation

mydea
Copy link
Member

@mydea mydea commented Jan 10, 2024

This is a bit WIP, but does the following things:

  • Deprecate Transaction integration, because we don't really document this and it's not clear if we need it.
  • Update the type definitions for the legacy integration classes, because they were partially swallowed. There is some TS issue with inferring/using the types there, no idea why - but it always complains when they are used in another package or so... So I ended up type casting the integration classes & adding the constructor types manually where necessary. This should ensure that you can new MyIntegration(options) with type safety for options - which is IMHO the most important typing aspect of the integrations, because the rest is internal, really.
  • Make sure all integration functions use satisfies IntegrationFn to ensure they have correct types (^^ this lead to the issue above being highlighted, sadly)
    Export the integration functions from packages so users can use them

Missing some deprecation message, and maybe some cleanup... And some tests are failing right now.

@mydea mydea self-assigned this Jan 10, 2024
Copy link
Contributor

size-limit report 📦

Path Size
@sentry/browser (incl. Tracing, Replay, Feedback) - Webpack (gzipped) 76.96 KB (-0.04% 🔽)
@sentry/browser (incl. Tracing, Replay) - Webpack (gzipped) 68.35 KB (-0.02% 🔽)
@sentry/browser (incl. Tracing, Replay) - Webpack with treeshaking flags (gzipped) 61.98 KB (-0.03% 🔽)
@sentry/browser (incl. Tracing) - Webpack (gzipped) 32.37 KB (-0.05% 🔽)
@sentry/browser (incl. Feedback) - Webpack (gzipped) 30.96 KB (-0.06% 🔽)
@sentry/browser - Webpack (gzipped) 22.3 KB (-0.07% 🔽)
@sentry/browser (incl. Tracing, Replay, Feedback) - ES6 CDN Bundle (gzipped) 74.69 KB (+0.12% 🔺)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (gzipped) 66.33 KB (+0.12% 🔺)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (gzipped) 32.16 KB (+0.29% 🔺)
@sentry/browser - ES6 CDN Bundle (gzipped) 23.99 KB (+0.37% 🔺)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (minified & uncompressed) 208.93 KB (+0.12% 🔺)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (minified & uncompressed) 96.99 KB (+0.25% 🔺)
@sentry/browser - ES6 CDN Bundle (minified & uncompressed) 71.59 KB (+0.34% 🔺)
@sentry/browser (incl. Tracing) - ES5 CDN Bundle (gzipped) 35.13 KB (+0.25% 🔺)
@sentry/react (incl. Tracing, Replay) - Webpack (gzipped) 68.71 KB (-0.02% 🔽)
@sentry/react - Webpack (gzipped) 22.33 KB (-0.08% 🔽)
@sentry/nextjs Client (incl. Tracing, Replay) - Webpack (gzipped) 85.36 KB (-0.02% 🔽)
@sentry/nextjs Client - Webpack (gzipped) 49.48 KB (-0.04% 🔽)
@sentry-internal/feedback - Webpack (gzipped) 16.99 KB (-0.09% 🔽)

mydea added a commit that referenced this pull request Jan 16, 2024
This ensures that all integration classes have both the correct
constructor options, as well as the correct instance methods.

This is pretty dirty, but since we're going to remove the classes very
soon in v8, IMHO that's the easiest approach to make this work properly
for our users.

Somehow TS complains when using the option types directly, so we inline
them for now everywhere, which seems to make it happy. 🤷 soon we'll
remove these anyhow!

Extracted out of
#10143
@mydea mydea closed this Jan 16, 2024
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.

1 participant