Skip to content

Conversation

mydea
Copy link
Member

@mydea mydea commented Jan 9, 2025

This PR updates our code to use optional chaining, where possible.

This was mostly search-and-replace by xx && xx. regex, so there may def. be some remaining places, but this should cover a good amount of usage.

@mydea mydea requested review from Lms24 and andreiborza January 9, 2025 10:52
@mydea mydea self-assigned this Jan 9, 2025
@mydea mydea requested review from a team as code owners January 9, 2025 10:52
Copy link
Contributor

github-actions bot commented Jan 9, 2025

size-limit report 📦

Path Size % Change Change
@sentry/browser 23.07 KB -0.02% -4 B 🔽
@sentry/browser - with treeshaking flags 21.73 KB -0.17% -36 B 🔽
@sentry/browser (incl. Tracing) 35.55 KB -0.14% -48 B 🔽
@sentry/browser (incl. Tracing, Replay) 72.34 KB -0.06% -39 B 🔽
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 62.85 KB -0.12% -77 B 🔽
@sentry/browser (incl. Tracing, Replay with Canvas) 76.59 KB -0.06% -46 B 🔽
@sentry/browser (incl. Tracing, Replay, Feedback) 88.63 KB -0.08% -66 B 🔽
@sentry/browser (incl. Feedback) 39.34 KB -0.1% -37 B 🔽
@sentry/browser (incl. sendFeedback) 27.7 KB -0.08% -20 B 🔽
@sentry/browser (incl. FeedbackAsync) 32.51 KB -0.12% -39 B 🔽
@sentry/react 25.76 KB -0.07% -17 B 🔽
@sentry/react (incl. Tracing) 38.31 KB -0.13% -49 B 🔽
@sentry/vue 27.23 KB -0.12% -33 B 🔽
@sentry/vue (incl. Tracing) 37.37 KB -0.11% -41 B 🔽
@sentry/svelte 23.19 KB -0.06% -13 B 🔽
CDN Bundle 24.35 KB -0.11% -26 B 🔽
CDN Bundle (incl. Tracing) 35.86 KB -0.08% -27 B 🔽
CDN Bundle (incl. Tracing, Replay) 70.5 KB -0.08% -56 B 🔽
CDN Bundle (incl. Tracing, Replay, Feedback) 75.68 KB -0.1% -73 B 🔽
CDN Bundle - uncompressed 71.14 KB -0.2% -139 B 🔽
CDN Bundle (incl. Tracing) - uncompressed 106.51 KB -0.19% -200 B 🔽
CDN Bundle (incl. Tracing, Replay) - uncompressed 217.52 KB -0.11% -242 B 🔽
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 230.2 KB -0.14% -315 B 🔽
@sentry/nextjs (client) 38.44 KB -0.13% -48 B 🔽
@sentry/sveltekit (client) 36.09 KB -0.09% -33 B 🔽
@sentry/node 161.77 KB -0.02% -24 B 🔽
@sentry/node - without tracing 97.54 KB -0.05% -45 B 🔽
@sentry/aws-serverless 127.42 KB -0.02% -14 B 🔽

View base workflow run

@mydea mydea force-pushed the fn/replace-optional-chaining branch from 23e3a85 to aaae4ab Compare January 9, 2025 11:45
integration.afterAllSetup(client);

if (instrumentPageLoad && WINDOW && WINDOW.location) {
if (instrumentPageLoad && WINDOW.location) {
Copy link
Member

Choose a reason for hiding this comment

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

ah ok, I'm seeing there's a pattern where we no longer check on the definedness of WINDOW. I guess that's alright then? We should always get some kind of global object 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, I think some global object will always be there, so it should not be necessary to check this - or can we think of a scenario where this is necessary? 🤔

Copy link
Member

Choose a reason for hiding this comment

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

As long as we don't assume any browser-specific attributes to be present (which we don't in these changes) I think we're good!

@Lms24
Copy link
Member

Lms24 commented Jan 9, 2025

I would have expected the bundle size decrease to be a bit larger but every lost byte is a good byte 😅

@mydea
Copy link
Member Author

mydea commented Jan 9, 2025

I would have expected the bundle size decrease to be a bit larger but every lost byte is a good byte 😅

Yeah, had the same thought 😅

@billyvg
Copy link
Member

billyvg commented Jan 9, 2025

Is there a prefer optional chaining lint rule we could turn on?

@mydea
Copy link
Member Author

mydea commented Jan 9, 2025

Is there a prefer optional chaining lint rule we could turn on?

There is a rule actually, I will make a follow up PR adding this!

@mydea mydea merged commit 1048a43 into develop Jan 9, 2025
153 checks passed
@mydea mydea deleted the fn/replace-optional-chaining branch January 9, 2025 15:47
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.

5 participants