Skip to content

Conversation

andreiborza
Copy link
Member

⚠️ This is a breaking change ⚠️

Simplifies the package by getting rid of the @sentry/solidstart/middleware subexport in favor of a regular export.

The middleware now needs to be imported like this:

import { sentryBeforeResponseMiddleware } from '@sentry/solidstart';

@andreiborza andreiborza added the Package: solidstart Issues related to the Sentry SolidStart SDK label Aug 28, 2024
@andreiborza andreiborza requested review from Lms24, lforst and mydea August 28, 2024 09:21
Copy link
Contributor

github-actions bot commented Aug 28, 2024

size-limit report 📦

⚠️ Warning: Base artifact is not the latest one, because the latest workflow run is not done yet. This may lead to incorrect results. Try to re-run all tests to get up to date results.

Path Size % Change Change
@sentry/browser 22.52 KB - -
@sentry/browser (incl. Tracing) 34.85 KB - -
@sentry/browser (incl. Tracing, Replay) 71.21 KB - -
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 64.48 KB - -
@sentry/browser (incl. Tracing, Replay with Canvas) 75.57 KB - -
@sentry/browser (incl. Tracing, Replay, Feedback) 88.29 KB - -
@sentry/browser (incl. Tracing, Replay, Feedback, metrics) 90.13 KB - -
@sentry/browser (incl. metrics) 26.83 KB - -
@sentry/browser (incl. Feedback) 39.59 KB - -
@sentry/browser (incl. sendFeedback) 27.18 KB - -
@sentry/browser (incl. FeedbackAsync) 31.9 KB - -
@sentry/react 25.28 KB - -
@sentry/react (incl. Tracing) 37.83 KB - -
@sentry/vue 26.66 KB - -
@sentry/vue (incl. Tracing) 36.68 KB - -
@sentry/svelte 22.65 KB - -
CDN Bundle 23.77 KB - -
CDN Bundle (incl. Tracing) 36.53 KB - -
CDN Bundle (incl. Tracing, Replay) 70.86 KB - -
CDN Bundle (incl. Tracing, Replay, Feedback) 76.16 KB - -
CDN Bundle - uncompressed 69.61 KB - -
CDN Bundle (incl. Tracing) - uncompressed 108.28 KB - -
CDN Bundle (incl. Tracing, Replay) - uncompressed 219.66 KB - -
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 232.85 KB - -
@sentry/nextjs (client) 37.6 KB - -
@sentry/sveltekit (client) 35.44 KB - -
@sentry/node 115.71 KB - -
@sentry/node - without tracing 90.1 KB - -
@sentry/aws-serverless 99.51 KB - -

View base workflow run

Copy link
Member

@Lms24 Lms24 left a comment

Choose a reason for hiding this comment

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

I have a basic question: What was the reason we exported it as a sub-export previously and why do we not need to do it anymore? Also is there a concrete reason not to export the middleware as a sub-export?

I'm asking because I wanna make sure we're not accidentally importing something from a solid package in the middleware which has the potential to break otel instrumentation (we've had this in NestJS).

Copy link
Member

@Lms24 Lms24 left a comment

Choose a reason for hiding this comment

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

Discussed my previous comment offline, LGTM!

(For future readesrs: Main that the middleware doesn't have to be a sub-export as well as discoverability for users and simplification of build process)

@andreiborza
Copy link
Member Author

I have a basic question: What was the reason we exported it as a sub-export previously and why do we not need to do it anymore? Also is there a concrete reason not to export the middleware as a sub-export?

I'm asking because I wanna make sure we're not accidentally importing something from a solid package in the middleware which has the potential to break otel instrumentation (we've had this in NestJS).

As discussed, the subexport pattern has been first used for the solidrouter integration so that we can mark solid router as an optional dependency.

For the middleware, I previously thought we might want to keep it out of the main package but there really is no benefit here, it's simpler for users and explanation to just import this straight from the root.

@andreiborza andreiborza merged commit 2b3e26f into develop Aug 28, 2024
99 checks passed
@andreiborza andreiborza deleted the ab/solidstart-middleware-remove-subexport branch August 28, 2024 10:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Package: solidstart Issues related to the Sentry SolidStart SDK

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants