-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
ref(solidstart): Simplify middleware structure without subexport #13494
ref(solidstart): Simplify middleware structure without subexport #13494
Conversation
size-limit report 📦
|
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 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).
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.
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)
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. |
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: