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(node): Don't return after process.exit #4512

Merged
merged 1 commit into from
Feb 7, 2022

Conversation

AbhiPrasad
Copy link
Member

@AbhiPrasad AbhiPrasad commented Feb 7, 2022

Any code after process.exit(1) is unreachable, so this return statement does nothing. We can remove it safely.

Any code after `process.exit(1)` is unreachable, so this return
statement does nothing. We can remove it safely.
@AbhiPrasad AbhiPrasad requested review from a team and lobsterkatie and removed request for a team February 7, 2022 19:20
@github-actions
Copy link
Contributor

github-actions bot commented Feb 7, 2022

size-limit report

Path Base Size (193b2ec) Current Size Change
@sentry/browser - CDN Bundle (gzipped) 19.69 KB 19.69 KB +0.01% 🔺
@sentry/browser - CDN Bundle (minified) 62.91 KB 62.91 KB +0.01% 🔺
@sentry/browser - Webpack 22.21 KB 22.21 KB 0%
@sentry/browser - Webpack - gzip = false 76.02 KB 76.02 KB 0%
@sentry/react - Webpack 22.24 KB 22.24 KB 0%
@sentry/nextjs Client - Webpack 46.31 KB 46.31 KB 0%
@sentry/browser + @sentry/tracing - CDN Bundle (gzipped) 28.22 KB 28.23 KB +0.02% 🔺

Copy link
Member

@lobsterkatie lobsterkatie left a comment

Choose a reason for hiding this comment

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

TS is happy with this? Once or twice I've come across a stray useless return statement and it's gotten mad when I've pulled it out - "not all code paths return a value," etc, etc. Here I'd hope it'd be smart enough to recognize that exit ends things, but you never know...

UPDATE: I guess if all the checks are passing, it's fine with it. Will mash approve.

@AbhiPrasad
Copy link
Member Author

Yeah it was actually my vscode's TS that complained.

If we look at the node types, we can see that process.exit() returns a never: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/8a401fa673d1593f39151a16c1ca8e60bdee1e94/types/node/process.d.ts#L617, which should instruct the TS compiler that anything below that function call is unreachable code. This functionality was added in 3.7 microsoft/TypeScript#28859 (comment), so idk why it doesn't error. Also we didn't specify a value for allowUnreachableCode.

Might be missing something simple 🤷

@AbhiPrasad AbhiPrasad merged commit 52520ba into master Feb 7, 2022
@AbhiPrasad AbhiPrasad deleted the abhi-no-return-after-exit branch February 7, 2022 23:25
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.

None yet

2 participants