Skip to content
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

feat(aws-serverless): Fix tree-shaking for aws-serverless package #12017

Merged
merged 5 commits into from
May 14, 2024

Conversation

mydea
Copy link
Member

@mydea mydea commented May 14, 2024

The prior fix was incomplete, because we were still using getDefaultIntegrations() inside of node's init, so the deps where pulled in anyhow.

Furthermore, it seems that preserveModules: false for @sentry/node also prevented this from working as expected.

So this PR does two things:

  1. Set preserveModules: true so that tree-shaking can work as expected (😿 )
  2. Expose a new initWithoutDefaultIntegrations method from @sentry/node which AWS uses, which avoids including any integrations by default. You have to pass your own defaultIntegrations to it.

This is not the prettiest solution, but I couldn't think of anything much better 😬 I also added a size-limit entry to keep track of this.

@mydea mydea self-assigned this May 14, 2024
Copy link
Contributor

github-actions bot commented May 14, 2024

size-limit report 📦

Path Size
@sentry/browser 21.65 KB (0%)
@sentry/browser (incl. Tracing) 32.69 KB (0%)
@sentry/browser (incl. Tracing, Replay) 68.03 KB (0%)
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 61.43 KB (0%)
@sentry/browser (incl. Tracing, Replay with Canvas) 72.07 KB (0%)
@sentry/browser (incl. Tracing, Replay, Feedback) 84.08 KB (0%)
@sentry/browser (incl. Feedback) 37.57 KB (0%)
@sentry/browser (incl. sendFeedback) 26.21 KB (0%)
@sentry/browser (incl. FeedbackAsync) 30.57 KB (0%)
@sentry/react 24.33 KB (0%)
@sentry/react (incl. Tracing) 35.65 KB (+0.02% 🔺)
@sentry/vue 25.48 KB (0%)
@sentry/vue (incl. Tracing) 34.48 KB (0%)
@sentry/svelte 21.77 KB (0%)
CDN Bundle 24.13 KB (0%)
CDN Bundle (incl. Tracing) 34.06 KB (0%)
CDN Bundle (incl. Tracing, Replay) 67.73 KB (0%)
CDN Bundle (incl. Tracing, Replay, Feedback) 72.71 KB (0%)
CDN Bundle - uncompressed 70.98 KB (0%)
CDN Bundle (incl. Tracing) - uncompressed 101.02 KB (0%)
CDN Bundle (incl. Tracing, Replay) - uncompressed 210.64 KB (0%)
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 222.94 KB (0%)
@sentry/nextjs (client) 34.88 KB (0%)
@sentry/sveltekit (client) 33.25 KB (0%)
@sentry/node 145.53 KB (-1.29% 🔽)
@sentry/aws-serverless 128.11 KB (added)

Copy link
Collaborator

@timfish timfish left a comment

Choose a reason for hiding this comment

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

Looks good.

Seems your PR has hit the same Biome issue as mine!

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 think that's a good solution!

@mydea mydea merged commit 43d4d33 into develop May 14, 2024
98 checks passed
@mydea mydea deleted the fn/proper-aws-tree-shaking branch May 14, 2024 15:02
andreiborza pushed a commit that referenced this pull request May 16, 2024
…2017)

The prior fix was incomplete, because we were still using
`getDefaultIntegrations()` inside of node's `init`, so the deps where
pulled in anyhow.

Furthermore, it seems that `preserveModules: false` for `@sentry/node`
also prevented this from working as expected.

So this PR does two things:

1. Set `preserveModules: true` so that tree-shaking can work as expected
(😿 )
2. Expose a new `initWithoutDefaultIntegrations` method from
`@sentry/node` which AWS uses, which avoids including any integrations
by default. You have to pass your own `defaultIntegrations` to it.

This is not the prettiest solution, but I couldn't think of anything
much better 😬 I also added a size-limit entry to keep track of this.
Lms24 added a commit that referenced this pull request May 16, 2024
We previously adjusted our lambda layer auto initialization in
#12017. This
unfortunately changed the build output of the `awslambda-auto`
bootstrapping script which required a package that isn't included in the
layer (`@sentry/node`). This PR fixes the `awslambda-auto` file; local
testing showed no more imports from `@sentry/node`.

fixes #12074
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.

3 participants