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

ref(feedback): Refactor Feedback types into @sentry/types and reduce the exported surface area #11355

Merged
merged 34 commits into from
Apr 8, 2024

Conversation

ryan953
Copy link
Member

@ryan953 ryan953 commented Mar 31, 2024

You can read this commit-by-commit to watch the refactor unfold

The situation is: after #11342 is merged, all the code inside each of the packages/feedback/src/[core|modal|screenshot] will be split up into separate integrations. These integrations can't share the same src/types/*.ts definitions anymore. Therefore type definitions will need to live in @sentry/types instead.

This PR moves all the types, and since they'll be public now in that package, i refactored things to reduce the public surface area and improve names where possible.
The only non-type changes in here are where we move createDialog.ts and createInput.ts into their respective integration.ts files, no code changes at all.

Related to #11435

Copy link

codecov bot commented Mar 31, 2024

Bundle Report

Changes will decrease total bundle size by 561.53kB ⬇️

Bundle name Size Change
@sentry/types-cjs 35 bytes 0 bytes
@sentry-internal/replay-esm 306.46kB 0 bytes
@sentry-internal/replay-canvas-cjs 29.51kB 0 bytes
@sentry-internal/node-integration-tests-cjs 1.04kB 0 bytes
@sentry/browser-cjs 106.89kB 0 bytes
@sentry/bun-cjs 13.5kB 0 bytes
@sentry-internal/node-integration-tests-esm 888 bytes 0 bytes
@sentry-internal/replay-canvas-esm 29.43kB 0 bytes
@sentry/types-esm 35 bytes 0 bytes
@sentry/browser-esm 104.06kB 0 bytes
@sentry/aws-serverless-cjs 20.29kB 5.67kB ⬆️
@sentry/utils-cjs 174.93kB 0 bytes
@sentry/bun-esm 10.05kB 0 bytes
@sentry/utils-esm 170.39kB 0 bytes
@sentry/core-cjs 238.78kB 0 bytes
@sentry/core-esm 235.2kB 0 bytes
@sentry/vercel-edge-cjs 18.23kB 0 bytes
@sentry-internal/integration-shims-cjs 3.65kB 0 bytes
@sentry-internal/integration-shims-esm 2.99kB 0 bytes
@sentry-internal/feedback-cjs 65.77kB 38 bytes ⬇️
@sentry/vercel-edge-esm 16.13kB 0 bytes
@sentry-internal/tracing-cjs 65.73kB 0 bytes
@sentry/opentelemetry-cjs 70.22kB 0 bytes
@sentry-internal/feedback-esm 65.45kB 47 bytes ⬇️
@sentry-internal/tracing-esm 65.5kB 0 bytes
@sentry/opentelemetry-esm 69.19kB 0 bytes
@sentry-internal/replay-cjs 306.35kB 0 bytes
@sentry/node-cjs 332.66kB 0 bytes
@sentry/node-esm 329.26kB 0 bytes
@sentry/google-cloud-serverless-cjs (removed) 23.0kB ⬇️
@sentry/astro-cjs (removed) 27.13kB ⬇️
@sentry/vue-cjs (removed) 20.19kB ⬇️
@sentry/google-cloud-serverless-esm (removed) 19.16kB ⬇️
@sentry/wasm-cjs (removed) 5.2kB ⬇️
@sentry/remix-esm (removed) 48.23kB ⬇️
@sentry/vue-esm (removed) 18.85kB ⬇️
@sentry/nextjs-cjs (removed) 15.5kB ⬇️
@sentry/sveltekit-esm (removed) 61.08kB ⬇️
@sentry/gatsby-esm (removed) 385 bytes ⬇️
@sentry/nextjs-esm (removed) 12.53kB ⬇️
@sentry/sveltekit-cjs (removed) 69.31kB ⬇️
@sentry/remix-cjs (removed) 53.62kB ⬇️
@sentry/astro-esm (removed) 23.39kB ⬇️
@sentry/svelte-cjs (removed) 13.84kB ⬇️
@sentry/gatsby-cjs (removed) 905 bytes ⬇️
@sentry/profiling-node-cjs (removed) 25.5kB ⬇️
@sentry/profiling-node-esm (removed) 25.52kB ⬇️
@sentry/react-cjs (removed) 45.04kB ⬇️
@sentry/wasm-esm (removed) 4.85kB ⬇️
@sentry/svelte-esm (removed) 12.72kB ⬇️
@sentry/react-esm (removed) 41.18kB ⬇️

@ryan953 ryan953 requested review from billyvg, mydea, AbhiPrasad, c298lee and a team April 1, 2024 17:53
@ryan953 ryan953 requested a review from AbhiPrasad April 2, 2024 23:49
Copy link
Contributor

github-actions bot commented Apr 8, 2024

size-limit report 📦

Path Size
@sentry/browser 22.1 KB (0%)
@sentry/browser (incl. Tracing) 31.71 KB (0%)
@sentry/browser (incl. Tracing, Replay) 66.92 KB (0%)
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 60.52 KB (0%)
@sentry/browser (incl. Tracing, Replay with Canvas) 70.75 KB (0%)
@sentry/browser (incl. Tracing, Replay, Feedback) 75.67 KB (-0.01% 🔽)
@sentry/browser (incl. Feedback) 30.87 KB (-0.02% 🔽)
@sentry/browser (incl. Feedback, Feedback Modal) 30.88 KB (-0.02% 🔽)
@sentry/browser (incl. Feedback, Feedback Modal, Feedback Screenshot) 30.89 KB (-0.02% 🔽)
@sentry/browser (incl. sendFeedback) 26.88 KB (0%)
@sentry/react 24.78 KB (0%)
@sentry/react (incl. Tracing) 34.61 KB (0%)
@sentry/vue 25.53 KB (0%)
@sentry/vue (incl. Tracing) 33.44 KB (0%)
@sentry/svelte 22.23 KB (0%)
CDN Bundle 24.11 KB (0%)
CDN Bundle (incl. Tracing) 32.66 KB (0%)
CDN Bundle (incl. Tracing, Replay) 66.37 KB (0%)
CDN Bundle (incl. Tracing, Replay, Feedback) 71.63 KB (+0.01% 🔺)
CDN Bundle - uncompressed 71.67 KB (0%)
CDN Bundle (incl. Tracing) - uncompressed 97.71 KB (0%)
CDN Bundle (incl. Tracing, Replay) - uncompressed 207.1 KB (0%)
@sentry/nextjs (client) 33.8 KB (0%)
@sentry/sveltekit (client) 32.24 KB (0%)
@sentry/node 119.95 KB (0%)

@ryan953 ryan953 merged commit 7383f8a into develop Apr 8, 2024
96 checks passed
@ryan953 ryan953 deleted the ryan953/feedback-types branch April 8, 2024 21:37
cadesalaberry pushed a commit to cadesalaberry/sentry-javascript that referenced this pull request Apr 19, 2024
…the exported surface area (getsentry#11355)

You can read this commit-by-commit to watch the refactor unfold

The situation is: after
getsentry#11342 is merged, all
the code inside each of the
`packages/feedback/src/[core|modal|screenshot]` will be split up into
separate integrations. These integrations can't share the same
`src/types/*.ts` definitions anymore. Therefore type definitions will
need to live in @sentry/types instead.

This PR moves all the types, and since they'll be public now in that
package, i refactored things to reduce the public surface area and
improve names where possible.
The only non-type changes in here are where we move `createDialog.ts`
and `createInput.ts` into their respective `integration.ts` files, no
code changes at all.

Related to getsentry#11435
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