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(solidstart): Add autoInjectServerSentry: 'experimental_dynamic-import #14863

Merged
merged 21 commits into from
Jan 10, 2025

Conversation

s1gr1d
Copy link
Member

@s1gr1d s1gr1d commented Dec 30, 2024

⚠️ THIS PR IS BASED ON #14862

Adds the option to dynamically import the server config file.

// app.config.ts
import { defineConfig } from '@solidjs/start/config';
import { sentrySolidStartVite, withSentry } from '@sentry/solidstart';

export default defineConfig(withSentry(
    { /* ... */ },
    { 
      autoInjectServerSentry: 'experimental_dynamic-import'
    })
 );

@s1gr1d s1gr1d requested a review from andreiborza December 30, 2024 14:43
Copy link
Contributor

github-actions bot commented Dec 30, 2024

size-limit report 📦

Path Size % Change Change
@sentry/browser 22.74 KB - -
@sentry/browser - with treeshaking flags 21.51 KB - -
@sentry/browser (incl. Tracing) 35.39 KB - -
@sentry/browser (incl. Tracing, Replay) 72.09 KB - -
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 62.62 KB - -
@sentry/browser (incl. Tracing, Replay with Canvas) 76.36 KB - -
@sentry/browser (incl. Tracing, Replay, Feedback) 88.38 KB - -
@sentry/browser (incl. Feedback) 39.07 KB - -
@sentry/browser (incl. sendFeedback) 27.42 KB - -
@sentry/browser (incl. FeedbackAsync) 32.25 KB - -
@sentry/react 25.49 KB - -
@sentry/react (incl. Tracing) 38.15 KB - -
@sentry/vue 27.08 KB - -
@sentry/vue (incl. Tracing) 37.24 KB - -
@sentry/svelte 22.86 KB - -
CDN Bundle 24.12 KB - -
CDN Bundle (incl. Tracing) 35.7 KB - -
CDN Bundle (incl. Tracing, Replay) 70.25 KB - -
CDN Bundle (incl. Tracing, Replay, Feedback) 75.46 KB - -
CDN Bundle - uncompressed 70.47 KB - -
CDN Bundle (incl. Tracing) - uncompressed 106.04 KB - -
CDN Bundle (incl. Tracing, Replay) - uncompressed 216.92 KB - -
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 229.67 KB - -
@sentry/nextjs (client) 38.28 KB - -
@sentry/sveltekit (client) 35.9 KB - -
@sentry/node 161.47 KB +0.01% +1 B 🔺
@sentry/node - without tracing 97.28 KB +0.01% +1 B 🔺
@sentry/aws-serverless 127.12 KB - -

View base workflow run

Copy link

codecov bot commented Dec 30, 2024

❌ 3 Tests Failed:

Tests completed Failed Passed Skipped
12 3 9 0
View the top 3 failed tests by shortest run time
errors.server.test.ts server-side errorscaptures server action error
Stack Traces | 30s run time
errors.server.test.ts:5:3 captures server action error
performance.server.test.ts sends a server action transaction on pageload
Stack Traces | 30s run time
performance.server.test.ts:9:1 sends a server action transaction on pageload
performance.server.test.ts sends a server action transaction on client navigation
Stack Traces | 30s run time
performance.server.test.ts:32:1 sends a server action transaction on client navigation

To view more test analytics, go to the Test Analytics Dashboard
📢 Thoughts on this report? Let us know!

@s1gr1d s1gr1d force-pushed the sig/solidstart-dynamic-import branch from 6616ad1 to a13d2b6 Compare January 2, 2025 10:24
Comment on lines 24 to 26
if (options.autoInjectServerSentry !== 'experimental_dynamic-import') {
sentryPlugins.push(makeBuildInstrumentationFilePlugin(options));
}
Copy link
Member

Choose a reason for hiding this comment

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

m: I left this comment on the other PR as well, just to ensure we don't forget about it here: We copy the release injection files now, so we should add the plugin in a way that the instrumentation file also gets injected with debug ids.

@s1gr1d s1gr1d force-pushed the sig/solidstart-dynamic-import branch from a13d2b6 to 5ff2f0f Compare January 7, 2025 15:41
@s1gr1d s1gr1d requested a review from andreiborza January 7, 2025 15:41
@s1gr1d s1gr1d force-pushed the ab/solidstart-withsentry branch from 0d73fd9 to 60dacf6 Compare January 8, 2025 09:40
@s1gr1d s1gr1d force-pushed the ab/solidstart-withsentry branch from 60dacf6 to 333d210 Compare January 8, 2025 09:46
@s1gr1d s1gr1d force-pushed the sig/solidstart-dynamic-import branch from 5ff2f0f to f84bcf5 Compare January 8, 2025 09:46
Copy link
Member

@andreiborza andreiborza left a comment

Choose a reason for hiding this comment

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

LGTM

@s1gr1d s1gr1d marked this pull request as ready for review January 8, 2025 11:09
Base automatically changed from ab/solidstart-withsentry to develop January 8, 2025 11:10
# Conflicts:
#	packages/solidstart/src/config/addInstrumentation.ts
#	packages/solidstart/src/config/withSentry.ts
#	packages/solidstart/src/vite/sentrySolidStartVite.ts
#	packages/solidstart/src/vite/types.ts
#	packages/solidstart/test/config/addInstrumentation.test.ts
@s1gr1d s1gr1d merged commit 38ff6eb into develop Jan 10, 2025
128 checks passed
@s1gr1d s1gr1d deleted the sig/solidstart-dynamic-import branch January 10, 2025 15:06
s1gr1d added a commit that referenced this pull request Jan 22, 2025
…import` (#14863)

⚠️ THIS PR IS BASED ON
#14862

Adds the option to dynamically import the server config file.

```typescript
// app.config.ts
import { defineConfig } from '@solidjs/start/config';
import { sentrySolidStartVite, withSentry } from '@sentry/solidstart';

export default defineConfig(withSentry(
    { /* ... */ },
    {
      autoInjectServerSentry: 'experimental_dynamic-import'
    })
 );
```

---------

Co-authored-by: Andrei Borza <[email protected]>
(cherry picked from commit 38ff6eb)
s1gr1d added a commit that referenced this pull request Jan 23, 2025
…import` (#14863)

⚠️ THIS PR IS BASED ON
#14862

Adds the option to dynamically import the server config file.

```typescript
// app.config.ts
import { defineConfig } from '@solidjs/start/config';
import { sentrySolidStartVite, withSentry } from '@sentry/solidstart';

export default defineConfig(withSentry(
    { /* ... */ },
    {
      autoInjectServerSentry: 'experimental_dynamic-import'
    })
 );
```

---------

Co-authored-by: Andrei Borza <[email protected]>
(cherry picked from commit 38ff6eb)
s1gr1d added a commit that referenced this pull request Jan 23, 2025
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.

2 participants