Skip to content

chore: extra guards process.env @W-17994335#5281

Merged
wjhsf merged 1 commit intomasterfrom
wjh/extra-guard-process
Mar 10, 2025
Merged

chore: extra guards process.env @W-17994335#5281
wjhsf merged 1 commit intomasterfrom
wjh/extra-guard-process

Conversation

@wjhsf
Copy link
Contributor

@wjhsf wjhsf commented Mar 10, 2025

evidently some places have process but not process.env

Details

Does this pull request introduce a breaking change?

  • 😮‍💨 No, it does not introduce a breaking change.
  • 💔 Yes, it does introduce a breaking change.

Does this pull request introduce an observable change?

  • 🤞 No, it does not introduce an observable change.
  • 🔬 Yes, it does include an observable change.

GUS work item

evidently some places have `process` but not `process.env`
@wjhsf wjhsf requested a review from a team as a code owner March 10, 2025 16:24
if (
typeof process === 'object' &&
typeof process?.env === 'object' &&
process.env &&
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: if typeof process?.env === 'object', isn't it almost guaranteed that process.env is truthy, making this third check unnecessary? I think the only possible way for this to be falsy is if process.env == document.all right?

Copy link
Contributor Author

@wjhsf wjhsf Mar 10, 2025

Choose a reason for hiding this comment

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

It would be document.all or null. And it would be weird, but not entirely implausible, if somebody did process.env = null somewhere to avoid exposing env vars.

tbh we just made an evidently bad assumption that typeof process === 'object' implies process = { env : {} }, so now I don't wanna assume anything.

@wjhsf wjhsf merged commit d2368b6 into master Mar 10, 2025
11 checks passed
@wjhsf wjhsf deleted the wjh/extra-guard-process branch March 10, 2025 17:24
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