Skip to content

fix(replay): Fix ts errors with replay_type - #6681

Merged
mydea merged 3 commits into
masterfrom
fix-replay-fix-ts-replay-type
Jan 9, 2023
Merged

fix(replay): Fix ts errors with replay_type#6681
mydea merged 3 commits into
masterfrom
fix-replay-fix-ts-replay-type

Conversation

@billyvg

Copy link
Copy Markdown
Member

Fixes type error from #6658 (semantic merge issue).

Comment threadpackages/types/src/replay.ts Outdated
trace_ids: string[];
replay_id: string;
segment_id: number;
replay_type: 'session' | 'error';

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

This is of type ReplayRecordingMode, but that type is defined in replays/types and would probably cause circular import issues -- should we move that type into types package?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes, I think that makes sense! I'll move it there and get this PR merged (so we unblock master) 👍

const dsn = client?.getDsn();

if (!client || !scope || !transport || !dsn) {
if (!client || !scope || !transport || !dsn || !this.session || !this.session.sampled) {

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

This should never be the case in this method but needed for ts and got rid of optional chaining

@github-actions

github-actionsBot commented Jan 7, 2023

Copy link
Copy Markdown
Contributor

size-limit report 📦

PathSize
@sentry/browser - ES5 CDN Bundle (gzipped + minified)19.84 KB (-0.01% 🔽)
@sentry/browser - ES5 CDN Bundle (minified)61.46 KB (+0.03% 🔺)
@sentry/browser - ES6 CDN Bundle (gzipped + minified)18.62 KB (+0.01% 🔺)
@sentry/browser - ES6 CDN Bundle (minified)55 KB (+0.02% 🔺)
@sentry/browser - Webpack (gzipped + minified)20.38 KB (-0.05% 🔽)
@sentry/browser - Webpack (minified)66.55 KB (-0.14% 🔽)
@sentry/react - Webpack (gzipped + minified)20.4 KB (-0.03% 🔽)
@sentry/nextjs Client - Webpack (gzipped + minified)47.67 KB (-0.03% 🔽)
@sentry/browser + @sentry/tracing - ES5 CDN Bundle (gzipped + minified)26.82 KB (+0.03% 🔺)
@sentry/browser + @sentry/tracing - ES6 CDN Bundle (gzipped + minified)25.25 KB (0%)
@sentry/replay ES6 CDN Bundle (gzipped + minified)43 KB (+0.03% 🔺)
@sentry/replay - Webpack (gzipped + minified)38.25 KB (+0.04% 🔺)

@mydea
mydeaforce-pushed the fix-replay-fix-ts-replay-type branch from d7c84b5 to 6cdcd81CompareJanuary 9, 2023 08:41

@mydeamydea left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I took this over in order to get this merged, hope that's ok @billyvg :)

@mydeamydea self-assigned this Jan 9, 2023
@mydea
mydea enabled auto-merge (squash) January 9, 2023 08:50
@mydea
mydeaforce-pushed the fix-replay-fix-ts-replay-type branch from 017b763 to e120865CompareJanuary 9, 2023 09:12
@mydea
mydea merged commit 012ff34 into masterJan 9, 2023
@mydea
mydea deleted the fix-replay-fix-ts-replay-type branch January 9, 2023 09:32
Sign up for freeto 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

@billyvg@mydea@Lms24