-
Notifications
You must be signed in to change notification settings - Fork 36
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(core): Only inject release into entrypoints per default #166
Conversation
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 like this approach. Seems to be a lot simpler than what we had before. I guess this is also kinda "behaviorally" breaking, so let's maybe add a migration note as well?
lmk when this is ready to ✅
@@ -253,15 +219,22 @@ const unplugin = createUnplugin<Options>((options, unpluginMetaContext) => { | |||
* @param id Always the absolute (fully resolved) path to the module. | |||
* @returns transformed code + source map | |||
*/ | |||
transform(code, id) { | |||
async transform(code, id) { | |||
logger.debug('Called "transform":', { id }); | |||
|
|||
// The MagicString library allows us to generate sourcemaps for the changes we make to the user code. | |||
const ms = new MagicString(code); | |||
|
|||
// Appending instead of prepending has less probability of mucking with user's source maps. |
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.
Is this comment still valid/necessary?
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.
(referring to line 228)
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.
Yup thanks! :)
@Lms24 should be ready for review! |
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.
Nice, let's go!
…-into-entrypoints
After upgrading from 0.3.0 to 0.4.0, it seems that release is not injected property.
Didn't have any issue on v0.3.0. I am using vite to bundle the assets.
X-posted in #178 |
Fixes #163
Turns out our approach for injecting the release value sucks in case you want to preserve modules while transpiling because we'll create a throwing
require("\0sentry-release-injector")
statement since\0sentry-release-injector
is only virtual and you can't "preserve" a virtual file.This PR is fixing this by slightly changing how the
releaseInjectionTargets
option works. By default we'll inject the release into all entry points now (as opposed to all modules) - without any import magic - just by injecting code. This may have bundle size impact but users can still control how this works by providing a function toreleaseInjectionTargets
.