ref: Use optional chaining wherever possible #17017
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
size-limit report 📦
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Bug: Refactor Changes `ctx` Assignment and Call Behavior
The refactoring of the ctx assignment introduces two behavioral changes:
- Fallback and type guarantee:
ctxis no longer guaranteed to be an object. Previously, it defaulted to{}whenvercelRequestContextGlobal.get()returned a falsy value. Now,ctxwill beundefinedor the falsy value returned byget(), which can alter the subsequentctx?.waitUntilcheck. - Function call count: The original code called
vercelRequestContextGlobal.get()twice when it returned a truthy value, while the new code calls it only once. This changes behavior ifget()has side effects or returns different values on subsequent calls.
packages/core/src/utils/vercelWaitUntil.ts#L20-L21
sentry-javascript/packages/core/src/utils/vercelWaitUntil.ts
Lines 20 to 21 in 250d7fc
Was this report helpful? Give feedback by reacting with 👍 or 👎
Lms24
reviewed
Jul 15, 2025
| @@ -18,8 +18,7 @@ export function vercelWaitUntil(task: Promise<unknown>): void { | |||
| // @ts-expect-error This is not typed | |||
| GLOBAL_OBJ[Symbol.for('@vercel/request-context')]; | |||
|
|
|||
| const ctx = | |||
| vercelRequestContextGlobal?.get && vercelRequestContextGlobal.get() ? vercelRequestContextGlobal.get() : {}; | |||
| const ctx = vercelRequestContextGlobal?.get?.(); | |||
There was a problem hiding this comment.
is this identical or should it rather be
Suggested change
| const ctx = vercelRequestContextGlobal?.get?.(); | |
| const ctx = vercelRequestContextGlobal?.get?.() ?? {} |
?
There was a problem hiding this comment.
it's not identical, but how this is used is:
if (ctx?.waitUntil) {
ctx.waitUntil(task);
}which should work the same I'd say?
There was a problem hiding this comment.
ah ok, sorry, should have checked the usage. Makes sense!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A future version of eslint flagged & auto-fixed these, extracting this out for easier review.