Skip to content

fix(astro): Ensure server-side exports work correctly#12453

Merged
Lms24 merged 2 commits intodevelopfrom
lms/fix-astro-server-exports
Jun 11, 2024
Merged

fix(astro): Ensure server-side exports work correctly#12453
Lms24 merged 2 commits intodevelopfrom
lms/fix-astro-server-exports

Conversation

@Lms24
Copy link
Copy Markdown
Member

@Lms24 Lms24 commented Jun 11, 2024 •
edited
Loading

This PR fixes a server-side re-export problem of @sentry/node exports in the Astro SDK.

Previously, our emitted ESM index.server.js file would contain these two re-exports:

export * from '@sentry/node'
export { /* explicit named exports */ } from '@sentry/node';

Unfortunately, it seems that with recent Astro (or SDK?) versions, this caused Sentry imports in .astro files to resolve to undefined rather than to their actual exports:

// example page.astro
---
import * as Sentry from '@sentry/astro';
Sentry.captureException(new Error('Hello, Sentry!'));
---

It seems that the export * from '@sentry/node' "overruled" the explicit exports. So this PR changes our export statements to:

  • only export explicit, named exports in the JS server side entry point
  • continue exporting all types via the * export in the types entry point

fixes #12410

@Lms24 Lms24 mentioned this pull request Jun 11, 2024
Closed
3 tasks
@Lms24 Lms24 self-assigned this Jun 11, 2024
lforst
lforst approved these changes Jun 11, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@lforst lforst left a comment

Choose a reason for hiding this comment

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

super odd

@arty-name
Copy link
Copy Markdown
Contributor

arty-name commented Jun 11, 2024

Thank you for the quick action!

If this behavior is specific to Astro (or maybe Vite used by Astro), do you think it should be reported upstream?

@Lms24 Lms24 enabled auto-merge (squash) June 11, 2024 09:06
@Lms24
Copy link
Copy Markdown
Member Author

Lms24 commented Jun 11, 2024

If this behavior is specific to Astro (or maybe Vite used by Astro), do you think it should be reported upstream?

To be honest, I'm not sure - it might also be related to our end, how we declare entry points in the package.json etc. It'd also fairly unusual for a library to provide this kind of isomorphic client and server side API. Perhaps there is a nicer fix out there but this one should unblock you as well as some other folks who reported this problem in different channels.

Before we report this, we'd need to figure out a minimal reproduction and take Sentry out of the equation as much as possible.

@andreiborza andreiborza mentioned this pull request Jun 11, 2024
Merged
@Lms24
Copy link
Copy Markdown
Member Author

Lms24 commented Jun 11, 2024 •
edited
Loading

Our "consistent node exports" e2e test just caught a bunch of additional missing exports for this change. Added them in cc1b529

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jun 11, 2024

size-limit report 📦

Path Size
@sentry/browser 22.04 KB (+0.16% 🔺)
@sentry/browser (incl. Tracing) 33.23 KB (+0.11% 🔺)
@sentry/browser (incl. Tracing, Replay) 68.95 KB (+0.05% 🔺)
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 62.27 KB (+0.06% 🔺)
@sentry/browser (incl. Tracing, Replay with Canvas) 73.02 KB (+0.05% 🔺)
@sentry/browser (incl. Tracing, Replay, Feedback) 85.14 KB (+0.05% 🔺)
@sentry/browser (incl. Tracing, Replay, Feedback, metrics) 86.98 KB (+0.05% 🔺)
@sentry/browser (incl. metrics) 26.23 KB (+0.14% 🔺)
@sentry/browser (incl. Feedback) 38.21 KB (+0.1% 🔺)
@sentry/browser (incl. sendFeedback) 26.63 KB (+0.16% 🔺)
@sentry/browser (incl. FeedbackAsync) 31.18 KB (+0.12% 🔺)
@sentry/react 24.81 KB (+0.15% 🔺)
@sentry/react (incl. Tracing) 36.27 KB (+0.11% 🔺)
@sentry/vue 26.05 KB (+0.14% 🔺)
@sentry/vue (incl. Tracing) 35.07 KB (+0.11% 🔺)
@sentry/svelte 22.17 KB (+0.16% 🔺)
CDN Bundle 23.39 KB (+0.15% 🔺)
CDN Bundle (incl. Tracing) 34.91 KB (+0.12% 🔺)
CDN Bundle (incl. Tracing, Replay) 69.02 KB (+0.07% 🔺)
CDN Bundle (incl. Tracing, Replay, Feedback) 74.15 KB (+0.06% 🔺)
CDN Bundle - uncompressed 68.71 KB (+0.12% 🔺)
CDN Bundle (incl. Tracing) - uncompressed 103.3 KB (+0.08% 🔺)
CDN Bundle (incl. Tracing, Replay) - uncompressed 213.75 KB (+0.04% 🔺)
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 226.21 KB (+0.04% 🔺)
@sentry/nextjs (client) 35.63 KB (+0.12% 🔺)
@sentry/sveltekit (client) 33.86 KB (+0.11% 🔺)
@sentry/node 111.93 KB (+0.01% 🔺)
@sentry/node - without tracing 89.4 KB (+0.01% 🔺)
@sentry/aws-serverless 98.46 KB (0%)

@Lms24 Lms24 merged commit cc7db73 into develop Jun 11, 2024
@Lms24 Lms24 deleted the lms/fix-astro-server-exports branch June 11, 2024 09:23
This was referenced Jul 3, 2024
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mydea mydea Awaiting requested review from mydea

@AbhiPrasad AbhiPrasad Awaiting requested review from AbhiPrasad

@andreiborza andreiborza Awaiting requested review from andreiborza

1 more reviewer

@lforst lforst lforst approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@Lms24 Lms24

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Regression: @sentry/astro stopped re-exporting captureException in 8.8.0

3 participants

@Lms24 @arty-name @lforst